Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: generalize how ads are created #863

Merged
merged 22 commits into from
Aug 29, 2023
Merged

Conversation

IanKrieger
Copy link
Member

@IanKrieger IanKrieger commented Aug 15, 2023

In order to prepare for more ads, need to generalize how ads are created.

Create

Screenshare.-.2023-08-25.4_21_39.PM.mp4

Edit

Screenshare.-.2023-08-28.9_33_08.AM.mp4

@IanKrieger IanKrieger marked this pull request as ready for review August 17, 2023 21:28
@IanKrieger IanKrieger requested a review from a team August 17, 2023 21:28
Copy link
Collaborator

@tackley tackley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks fine, and this looks like a great move forward.

But maybe there's things we can do to simplify the UI?

  1. could the page that lists all creatives show previews rather than listing the entires in a table? Also likely the target url is a key thing to show for those creatives

  2. the "import ad" behaviour feels a bit odd to me. I get a drop down I can tick entries in, which then shows up as chips in the autocomplete. But then they also appear at the top of the screen. And then I have to pick them from another autocomplete on the next page.

Would it work better to do something like:

  • click the "choose a previously saved creative" button
  • you get to see a list of all creatives (ideally as previews - perhaps using exactly the same component on the creative list page at top level?)
  • I click "add" next to one of those, it disappears from that list and appears in the the top list

But then there's the wider question of what this list of creatives represents and I can't really build a sensible mental model to represent that. As I understand it, this in fact just controlling the set of creatives shown in the autocomplete on the adset page, correct?

So if I realise the creative I want is not in that list, I have to come back to this page, add it, then it'll show up in the list? That feels awkward.

Happy to iterate on this, but maybe a brainstorming session would be worthwhile?

src/components/Creatives/CreativeAutocomplete.tsx Outdated Show resolved Hide resolved
@IanKrieger IanKrieger marked this pull request as draft August 18, 2023 21:01
@IanKrieger IanKrieger changed the title feat: allow use of existing creatives chore: generalize how ads are created Aug 23, 2023
@IanKrieger IanKrieger marked this pull request as ready for review August 23, 2023 21:35
@IanKrieger IanKrieger force-pushed the f/allow-existing-creatives branch 3 times, most recently from 4460f72 to 4da05b3 Compare August 25, 2023 20:51
@IanKrieger IanKrieger merged commit f72cb1c into master Aug 29, 2023
6 checks passed
@IanKrieger IanKrieger deleted the f/allow-existing-creatives branch August 29, 2023 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants