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

Minor button styling adjustments #88

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

alexkb0009
Copy link
Collaborator

  • Changes some things in FacetList to be literal buttons (+ styling updates for it), such as the 'view more' buttons and facets' titles for better accessibility (i.e. ability to use keyboard interaction ([shift+]tab/enter/space) w. it).

  • (No Effect) Adds 'fixed-height' class to some buttons which only have icon(s) as child (and no text); not used right now / may come back to later. On 4DN, there's a style rule in bootstrap_overrides.scss that sets >i:only-child to have line-height: inherit; this seems to work decently (but not perfectly; not used if 2+ icons in same button..) which could use some testing on CGAP-side.

And migrate to using d-flex align-items-center to position it.
N.B. icon line-height inherit helps with icon centering/alignment, also.
@utku-ozturk
Copy link
Member

utku-ozturk commented Nov 24, 2021

@alexkb0009 I have tested locally and looks good to me! My only concern is that facet title looks darker and larger. I think, h5.facet-title to button.facet-title transition in scss/facet-list.scss breaks the styling. Here is the old and new look and feel:

https://i.gyazo.com/39e54ea216a89ff92b7dbd5b8208df66.png

https://i.gyazo.com/7cc289c054bc5d864e4902d9ae91ef44.png

Copy link
Member

@Bianca-Morris Bianca-Morris left a comment

Choose a reason for hiding this comment

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

Tested this on local CGAP, and it looks good (would be good to double check on 4DN re: Utku's concerns above).

I will say, though, that if the goal of making the switch to buttons is to get tab-navigation working, there's still a lot of work to be done in the FacetList component (there are some issues with how the dropdowns behave atm). Anyway, this is a step in the right direction, for sure.

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.

3 participants