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

Add control for marking captures as starred #326

Merged
merged 7 commits into from
Nov 29, 2023
Merged

Conversation

annavik
Copy link
Member

@annavik annavik commented Nov 27, 2023

Not starred capture:
Screenshot 2023-11-27 at 23 25 14

Starred capture (when possible to unmark as starred, we can have this clickable):
Screenshot 2023-11-27 at 23 25 25

@annavik annavik requested a review from mihow November 27, 2023 22:28
Copy link

netlify bot commented Nov 27, 2023

Deploy Preview for ami-storybook ready!

Name Link
🔨 Latest commit 98b0851
🔍 Latest deploy log https://app.netlify.com/sites/ami-storybook/deploys/6565fda94f720f0008f987ce
😎 Deploy Preview https://deploy-preview-326--ami-storybook.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Nov 27, 2023

Deploy Preview for ami-web ready!

Name Link
🔨 Latest commit 98b0851
🔍 Latest deploy log https://app.netlify.com/sites/ami-web/deploys/6565fda9a496770008342296
😎 Deploy Preview https://deploy-preview-326--ami-web.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 84
Accessibility: 100
Best Practices: 92
SEO: 92
PWA: 70
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@mihow
Copy link
Collaborator

mihow commented Nov 28, 2023

@annavik I added an unstar endpoint and attempted to implement it in the UI. I also tried to use a tooltip instead of a label. But looks like label is required on a Button, so maybe there is a better way to do that for accessibility?

Let me know how I did.

Also what do you think about moving the heart to the main controls row, before the user clicks expand?

@annavik
Copy link
Member Author

annavik commented Nov 28, 2023

@annavik I added an unstar endpoint and attempted to implement it in the UI. I also tried to use a tooltip instead of a label. But looks like label is required on a Button, so maybe there is a better way to do that for accessibility?

Let me know how I did.

Also what do you think about moving the heart to the main controls row, before the user clicks expand?

Looks great! We have a separate IconButton component, I set it up like that to not make regular button component too complex. PR is updated!

Copy link
Collaborator

@mihow mihow left a comment

Choose a reason for hiding this comment

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

I love it. Also the loading indicator works great, thank you.

@mihow mihow merged commit e8bbbb7 into main Nov 29, 2023
5 checks passed
@mihow mihow deleted the web-ui-starred-captures branch February 8, 2024 21:03
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