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

fix(FilterBar): hide clear all button when there's nothing to clear #5322

Merged
merged 19 commits into from
Nov 28, 2024

Conversation

akanshaG0102
Copy link
Contributor

@akanshaG0102 akanshaG0102 commented Nov 25, 2024

Why

https://cultureamp.atlassian.net/browse/KZN-2858

What

  • Added new property to state to tell us whether the filter bar has any removable filters
  • Add a new property to filter bar context to tell us whether there's anything that can be cleared (either values or a removable filter)
  • Hide the 'Clear all' button when there's nothing to clear
  • Move tests around 'clear all' to storybook instead of vitest which struggled to know if the button was visible or not.
  • When removing filters, delete from object instead of setting to undefined
  • Avoid running update callback in FilterDateRangePicker on first load when values are empty

Copy link

changeset-bot bot commented Nov 25, 2024

🦋 Changeset detected

Latest commit: 03bc812

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@kaizen/components Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@dougmacknz dougmacknz changed the title Fix(Filterbar): Disable clearall when no active filter fix(FilterBar): disable clear all button when there's nothing to clear Nov 25, 2024
@dougmacknz dougmacknz marked this pull request as ready for review November 25, 2024 22:44
@dougmacknz dougmacknz requested a review from a team as a code owner November 25, 2024 22:44
@dougmacknz
Copy link
Contributor

(putting in review for chromatic)

Copy link
Contributor

github-actions bot commented Nov 25, 2024

✨ Here is your branch preview! ✨

Last updated for commit 03bc812: Switch toBeNull to not.toBeInTheDocument

@dougmacknz dougmacknz force-pushed the KZN-2858/disable-clearAll-btn-filter-bar branch from 5c5e6b7 to 6b68fa1 Compare November 25, 2024 23:14
@dougmacknz dougmacknz changed the title fix(FilterBar): disable clear all button when there's nothing to clear fix(FilterBar): hide clear all button when there's nothing to clear Nov 26, 2024
@dougmacknz dougmacknz force-pushed the KZN-2858/disable-clearAll-btn-filter-bar branch from 3d0fcff to e817bc8 Compare November 26, 2024 01:55
@HeartSquared
Copy link
Contributor

HeartSquared commented Nov 26, 2024

Seems to be a little buggy

Screen.Recording.2024-11-26.at.4.50.21.PM.mov

@dougmacknz dougmacknz force-pushed the KZN-2858/disable-clearAll-btn-filter-bar branch from dd1f4e3 to 5075111 Compare November 27, 2024 00:56
@dougmacknz dougmacknz force-pushed the KZN-2858/disable-clearAll-btn-filter-bar branch from 99e3d5f to 1d5b82c Compare November 28, 2024 04:34
@dougmacknz dougmacknz force-pushed the KZN-2858/disable-clearAll-btn-filter-bar branch from 1d5b82c to 03bc812 Compare November 28, 2024 04:42
@dougmacknz dougmacknz merged commit 7903e38 into main Nov 28, 2024
19 checks passed
@dougmacknz dougmacknz deleted the KZN-2858/disable-clearAll-btn-filter-bar branch November 28, 2024 22:00
Zystix pushed a commit that referenced this pull request Dec 2, 2024
…5322)

* fix(FilterBar): disable clearall if no active filters

* add changeset

* add isClearable into the context

* update and add new test for disable functionality

* Hide button instead of disabling

* Make isClearable check more efficient

* Add test for hasRemovableFilter

* Adjust changeset message

* Fix test

* Add SB tests for clear all and remove vitest equivalents

* Add extra step to test

* delete from object instead of setting to undefined

* Don't update value in FDRP on load if empty

* Add workaround for DateRangePicker in isClearable

* Adjust story names

* Split test stories up

* Use toBeNull() instead of toBe(null)

* Check value is not undefined

* Switch toBeNull to not.toBeInTheDocument

---------

Co-authored-by: Doug MacKenzie <[email protected]>
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