-
Notifications
You must be signed in to change notification settings - Fork 90
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(button): add vote button variant #1685
base: develop
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for stacks ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@dancormier @CGuindon What do you think about "dog feed" this new button styles in our docs and replace the existing feedback buttons (see screenshot)? |
@CGuindon @dancormier I am not sure how to feel about the focus state changing the internal background color of the button with different colors depending if it is selected or not. Would not be simpler and more intuitive to treat this special vote button like we do toggles or tags? I mean just surrounding the element with an outline and leave the background colors unchanged. I am thinking that if I struggle to understand which color should the vote button be when focused and I work in Stacks I can only imagine how difficult it would be for others to wrap their heads around it. |
@dancormier Could we remove the arrow icon button example in the icons section of the page? I think it could be confusing for people since now we have a dedicate vote variant. |
|
||
<div class="stacks-preview"> | ||
{% highlight html %} | ||
<button class="s-btn s-btn__vote" type="button"> |
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.
I think we should add the appropriate aria attributes to this example since it is used in most places as a toggle: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-pressed
{% highlight html %} | ||
<button class="s-btn s-btn__vote" type="button"> | ||
@Svg.ArrowUp | ||
<span class="v-visible-sr">Upvote</span> |
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.
Would it not be more elegant and shorter to use an aria-label
in the button instead?
@dancormier @CGuindon Is
|
@dancormier @CGuindon I think this is a good starting point to standardize the vote (or round or whatever 🙂) buttons in Stacks but I think it could benefit from us reflecting some more on our choices before shipping it. We certainly should not treat this work as "side" work especially because I imagine it will be quite painful to integrate and test this changes across different places in Core. I would recommend to plan this extraction work for one of our next sprints and address anything urgent in Core with a temporary fix. We can talk more in our planning today and it's totally ok if you disagree. 🙂 |
This PR adds a
vote
variant of the button component to more tightly control and customize vote button styling via Stacks.Deploy preview: https://deploy-preview-1685--stacks.netlify.app/product/components/buttons/#vote