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

[WDTK] Improved visual elements for button classes #1918

Closed
wants to merge 5 commits into from

Conversation

lucascumsille
Copy link
Contributor

@lucascumsille lucascumsille commented Sep 10, 2024

Relevant issue(s)

No issue, the default state of the tertiary button gave me the idea to make some improvements to some of the default and for all the hover states.

What does this do?

Currently the hover effect is quite subtle, which is not ideal for users with visual impairments. Ideally we want to change at least two properties when changing between states, to make it fairly obvious. For the tertiary button, I also changed the default state, to make more evident that is an interactive element. For this I increased the border-width, and changed the color(border and font) to what we use for links, which makes it more familiar for users that is an interactive element.

Screenshots

Default

Screen.Recording.2024-09-10.at.12.16.56.mov

Pro

Screen.Recording.2024-09-10.at.11.49.53.mov

Notes to reviewer

@lucascumsille lucascumsille requested a review from gbp September 10, 2024 10:15
@gbp
Copy link
Member

gbp commented Sep 10, 2024

@lucascumsille we also have a style of buttons for the Pro interface. Do we need an update there too:

image

@lucascumsille
Copy link
Contributor Author

@gbp Thank for letting me know =) I added an extra commit.

In case you are wonder, for the .button-tertiary and .button I could have simple swap their states between default and hover, the problem is both buttons are right next to each other they will look exactly the same while you are hovering over one. That is the reason why I added a different hover effect that does not resemble the button that is right next to it.

Currently the hover effect is quite subtle, which is not ideal for users with visual impairments. Ideally we want to change at least two properties when changing between states, to make it fairly obvious. For the tertiary button, I also changed the default state, to make more evident that is an interactive element. For this I increased the border-width, and changed the color(border and font) to what we use for links, which makes it more familiar for users that is an interactive element.
Copy link
Member

@gbp gbp left a comment

Choose a reason for hiding this comment

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

The hover transition for the primary button seems to jump to grey. It isn't as smooth as the other button types. Is it possible to fix this?

hover-transition.mov

@lucascumsille
Copy link
Contributor Author

@gbp I added a mixup that should solve the transition issue. Btw I erase some of the transitions that have been duplicated for hover effects, the bit transition is only needed it in the default and then it will get use on all the other states.

Copy link
Member

@gbp gbp left a comment

Choose a reason for hiding this comment

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

@lucascumsille this have merge conflicts and doesn't merge cleanly. I believe I've merged it locally by manually fixing the conflicts but need to check with you because some hover states which look strange:

homepage-make-a-request-hover.mov
nonpro-pagination-hover.mov
pro-pagination-hover.mov

@lucascumsille
Copy link
Contributor Author

@gbp No worries I'll do the merging, I think we are losing some stuff there.

@lucascumsille
Copy link
Contributor Author

Replaced by #1917

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