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

Refactored pill component to allow entire pill to be clickable; updat… #466

Merged
merged 1 commit into from
Oct 4, 2023

Conversation

wwhorton
Copy link
Contributor

…ed pill.less to preserve styling

There were some complaints about the filter pill having a hover state on the

  • and also a hover state on the close button, which was just the 'X'. The dual hover had been removed, but there was a request to make the entire pill clickable. This slightly refactors the component to make the button element be the entire pill, and adds some minor tweaks to pill.less to preserve the styling.

    Changes

    • puts contents of li in button in pill.js
    • adds .cf-icon-svg to selector for the styling that previously only applied to the button element

    Testing

    • yarn test should pass
    • From the CCDB Search page, add a filter and note the pill.
    • Hover over the pill and verify the state displays correctly whether you're over the 'X' or not
    • Click anywhere on the pill and verify that it is removed and the filter updates accordingly

    Checklist

    • Changes are limited to a single goal (no scope creep)
    • Code can be automatically merged (no conflicts)
    • Code follows the standards laid out in the front end playbook
    • Passes all existing automated tests
    • New functions include new tests
    • New functions are documented (with a description, list of inputs, and expected output)
    • Placeholder code is flagged
    • Visually tested in supported browsers and devices
    • Project documentation has been updated (including the "Unreleased" section of the CHANGELOG)
  • @anselmbradford
    Copy link
    Member

    Deferring review of this one to @contolini and @BrewCityBoy, now that they're on the team.

    Copy link
    Member

    @contolini contolini left a comment

    Choose a reason for hiding this comment

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

    Huge improvement 👍

    @wwhorton wwhorton added this pull request to the merge queue Oct 4, 2023
    Merged via the queue into main with commit 6ab42fe Oct 4, 2023
    2 checks passed
    @wwhorton wwhorton deleted the ww-filter-button-hover branch October 4, 2023 16:27
    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