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

[LIBSEARCH-1031] ACCOUNT: Update Settings display to match Figma diagram #322

Conversation

erinesullivan
Copy link
Collaborator

Overview

This pull request was originally focused on LIBSEARCH-1031, but the changes were already made. The focus then shifted to CSS and icon changes:

  • Used native CSS to style the radio buttons, and removed the need for the icons.
  • Used CSS for the arrow icon in the dropdown and removed the need for JavaScript for styling.
  • Updated the Material Icons import to only grab the necessary icons instead of the whole font family, improving load time.
  • Removed unnecessary CSS files.

Much more could be done, but this was already way out of scope.

Testing

  • Run the tests to make sure they pass (docker-compose run --rm web bundle exec rspec).
  • Make sure the PR is consistent in these browsers:
    • Chrome
    • Firefox
    • Safari
    • Edge
  • Run accessibility tests:
    • WAVE
    • ARC Toolkit
    • axe DevTools
  • Poke around the site to see if anything is broken.
    • Do the icons load faster?

Copy link
Collaborator

@niquerio niquerio left a comment

Choose a reason for hiding this comment

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

I deployed this to the workshop and see all of the warnings. Presumably they can be fixed up?

Screenshot from 2025-01-29 11-25-08

Copy link
Collaborator

@niquerio niquerio left a comment

Choose a reason for hiding this comment

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

In a different tab in the same browser I'm not seeing those warnings so, let's not worry about it after all.

@niquerio niquerio merged commit fa62f7e into main Jan 29, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants