-
Notifications
You must be signed in to change notification settings - Fork 4
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: vote received modal #533
Conversation
…rd into feat/vote-received-modal
…rd into feat/vote-received-modal
...s/Pages/Collections/Components/CollectionsVoteReceivedModal/CollectionsVoteReceivedModal.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you ran the images through the tinypng compression? 400 KB and 500 KB seems kinda "large"
...s/Pages/Collections/Components/CollectionsVoteReceivedModal/CollectionsVoteReceivedModal.tsx
Outdated
Show resolved
Hide resolved
...s/Pages/Collections/Components/CollectionsVoteReceivedModal/CollectionsVoteReceivedModal.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems this aspect of the spec is missing when sharing on X
<collection> - either the twitter username (@collection) if they have one, otherwise turn it into a tag based on the collection name (all lowercase, no spaces, e.g. #alphadogs)
@shahin-hq @crnkovic @samharperpittam feedback handled, added a second button to test a dummy collection with twitter account and without twitter account |
Summary
Closes
https://app.clickup.com/t/86dqnpgd9
note: this pr adds a temporal button that can be used to open the modal, since the interaction that eventually are going to trigger that modal are not ready
Checklist