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

feat: add logic that lets users favorite a template #9713

Merged
merged 28 commits into from
May 20, 2024

Conversation

nickoferrall
Copy link
Contributor

Fix #9115

Loom demo: https://www.loom.com/share/d59162967e244522aa40b73290a9c9a8

AC

  • Can favourite a template and see it show up in your favourites section
  • Can un-favourite a template
  • Can favourite and unfavourite a template from the template details view

@github-actions github-actions bot requested a review from tianrunhe May 3, 2024 16:21
@github-actions github-actions bot added the size/m label May 3, 2024
@nickoferrall nickoferrall requested review from Dschoordsch and removed request for tianrunhe May 3, 2024 16:36
Copy link
Contributor

@Dschoordsch Dschoordsch left a comment

Choose a reason for hiding this comment

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

The heart seems a bit misaligned for me (Safari, Mac), slightly too high on the card and more so on the category. It also conflicts with the "custom" badge on master, but we could remove that one again.
image

packages/server/graphql/public/types/User.ts Outdated Show resolved Hide resolved
"""
The user who's favorite templates were updated
"""
user: User!
Copy link
Contributor

Choose a reason for hiding this comment

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

+1 Seems excessive to return the full user. I think it would be enough to return the updated template and favourite state.

@nickoferrall
Copy link
Contributor Author

I've moved the heart in the card to be slightly lower and to the right. For the custom label, we have a separate discussion in Slack around that.

In the category labels, I think it is centred:

Screenshot 2024-05-07 at 17 46 18

Screenshot 2024-05-07 at 17 44 49 (2)

Copy link
Contributor

@Dschoordsch Dschoordsch left a comment

Choose a reason for hiding this comment

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

I don't want to be picky, but the category heart is offset 2px to the top
image
image

We also need to do something about the custom label first before merging.

@nickoferrall
Copy link
Contributor Author

For the custom label, once you and Terry have reached a decision in the Slack discussion, I can update this PR or wait for the "Custom" label to be reverted/changed.

I don't want to be picky, but the category heart is offset 2px to the top

On Chrome, it was centered for me, but I can see the 2px offset on Safari. I've pushed an update and now it's centered on Safari for me, too:

Screenshot 2024-05-08 at 18 00 38 Screenshot 2024-05-08 at 18 00 25

Base automatically changed from feat/9114/favorite-activities-ui to master May 20, 2024 17:12
@nickoferrall nickoferrall merged commit 4558e14 into master May 20, 2024
6 checks passed
@nickoferrall nickoferrall deleted the feat/9115/al-favorite-logic branch May 20, 2024 18:11
@github-actions github-actions bot mentioned this pull request May 21, 2024
24 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Activity Library v2: favourite activities logic
2 participants