Skip to content

feat(EuiFilterButton): accept percentages in numActiveFilters prop #8705

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.

Sign up for GitHub

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

Jump to bottom
Merged
tkajtoch merged 5 commits into from
Jun 3, 2025
Merged

feat(EuiFilterButton): accept percentages in numActiveFilters prop #8705

tkajtoch merged 5 commits into from
Jun 3, 2025

Conversation

@tkajtoch
Copy link
Member

@tkajtoch tkajtoch commented May 19, 2025 •
edited by mgadewoll
Loading

Summary

Resolves #8695

This PR extends the type of numActiveFilters to accept percentage values as strings. For simplicity, there's no fancy string type checking - instead, we trust the user and save a little bit of time during type resolution.

QA

  • Open Storybook and confirm percentage values like "20%" are accepted in numActiveFilters prop
  • Confirm there are no type errors when passing a percentage value string to numActiveFilters prop

General checklist

  • Browser QA
    • Checked in Chrome, Safari, Edge, and Firefox
    • Checked for accessibility including keyboard-only and screenreader modes
  • Docs site QA
  • Release checklist
    • A changelog entry exists and is marked appropriately.

@tkajtoch tkajtoch self-assigned this May 19, 2025
@tkajtoch tkajtoch force-pushed the feat/eui-filter-button-allow-percentage-values branch from 51dc13d to 431aa2a Compare May 30, 2025 08:48
@tkajtoch tkajtoch marked this pull request as ready for review May 30, 2025 09:04
@tkajtoch tkajtoch requested a review from a team as a code owner May 30, 2025 09:04
mgadewoll
mgadewoll reviewed May 30, 2025
View reviewed changes
packages/eui/src/components/filter_group/filter_button.tsx
* @example '20%'
*/
numActiveFilters?: number;
numActiveFilters?: number | string;
Copy link
Contributor

@mgadewoll mgadewoll May 30, 2025

Choose a reason for hiding this comment

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

Looking at Storybook, we'll need to set the control type. It currently (wrongly) falls back to object.
We either set it to only string numActiveFilters: { control: 'text' } (which also allows passing numbers - as string) or we'd need to use a radio group to provide fixed examples (example). I'd think using a text field is enough here.

Copy link
Member Author

@tkajtoch tkajtoch Jun 3, 2025

Choose a reason for hiding this comment

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

Fixed in b2dd13b

@elasticmachine
Copy link
Collaborator

elasticmachine commented Jun 3, 2025

💚 Build Succeeded

History

cc @tkajtoch

@elasticmachine
Copy link
Collaborator

elasticmachine commented Jun 3, 2025

💚 Build Succeeded

History

cc @tkajtoch

weronikaolejniczak
weronikaolejniczak reviewed Jun 3, 2025
View reviewed changes
packages/website/docs/components/forms/search-and-filter/filter-group.mdx
### Indicating number of filters

By passing a number to `numFilters` you can express the number of filters available. When the user has applied these filter add the prop `hasActiveFilters` as before and this will change the coloring of the indicator. You can also supply a number to `numActiveFilters`which will change the number displayed.
You can use the `numFilters` prop to display the total number of filters
Copy link
Contributor

@weronikaolejniczak weronikaolejniczak Jun 3, 2025

Choose a reason for hiding this comment

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

I like this wording much better, thanks for updating! 💚

weronikaolejniczak
weronikaolejniczak approved these changes Jun 3, 2025
View reviewed changes
Copy link
Contributor

@weronikaolejniczak weronikaolejniczak left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM! 🟢

Non-blocking suggestion, do we want to add some unit tests to test that the percentages work?

@tkajtoch
Copy link
Member Author

tkajtoch commented Jun 3, 2025

Non-blocking suggestion, do we want to add some unit tests to test that the percentages work?

In this case I decided to not add a test case for this since it's the same prop, just a slightly different native data type. There's no logic like "do this if type is number otherwise do that" so the existing cases cover pretty much everything already

@tkajtoch
Copy link
Member Author

tkajtoch commented Jun 3, 2025

I'm going to merge it as-is for the sake of time, but if you feel we should have an extra test case for this let's discuss this and add it separately :)

@tkajtoch tkajtoch merged commit 066de3f into elastic:main Jun 3, 2025
5 checks passed
@JasonStoltz JasonStoltz mentioned this pull request Jun 11, 2025
Merged
JasonStoltz added a commit to elastic/kibana that referenced this pull request Jun 17, 2025
@JasonStoltz @mgadewoll @elasticmachine
Upgrade EUI to v103.0.0 (#223414)
61ea2f3 
`102.3.0` ⏩ `103.0.0`

[Questions? Please see our Kibana upgrade
FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)

## Changes

[#8736](elastic/eui#8736),
[#8732](elastic/eui#8732), and
[#8732](elastic/eui#8732) include a number of
small style tweaks `EuiResizableCollapseButton`, `EuiTitle`, and
`EuiListGroupItem`.

[#8756](elastic/eui#8756) and
[#8744](elastic/eui#8744) rename a couple of
icons:

`questionInCircle` -> `question`
`iInCircle` > `info`

In both cases, the old name is **backwards-compatible** (unless
importing directly from `assets` folder (see more in the release notes
regarding that). However, the old name will eventually be deprecated as
a part of a larger set of deprecations.

**No action is required at this time to handle this renaming. We will
issue a dedicated PR to Kibana to replace the old names with the new
names in the future**

[#8725](elastic/eui#8725) adds Sky Blue, Yellow,
and Orange palettes to EUI's [color
palettes](https://eui.elastic.co/docs/utilities/color-palettes/#recommended-quantitative-palettes),
as well as new tokens for Risk, Neutral, and Warning.

**[Breaking change]** Please note that as part of this change, the
following tokens have been renamed:
`euiColorVisNeutral0` -> `euiColorVisBase0`
`euiColorVisWarning0` -> `euiColorVisWarning1`

The original tokens still exist, but have been updated to use new colors
in a way that would be breaking. This PR updates the existing references
in 0b21d2d.

## Package updates

### `@elastic/eui`

## [`v103.0.0`](https://github.com/elastic/eui/releases/v103.0.0)

- Replaced `question` icon glyph
([#8756](elastic/eui#8756))
- Updated `EuiResizableCollapseButton` to use an empty button
([#8736](elastic/eui#8736))
- Added `info` icon glyph
([#8744](elastic/eui#8744))
- Removed uppercase styling from `EuiText` `h6` headings to match
`EuiTitle` ([#8732](elastic/eui#8732))
- Removed heavier font weight from `xs` and `s` `EuiListGroupItem` sizes
for consistency ([#8732](elastic/eui#8732))
- Updated the `font-weight` of default `EuiFilterButton` and
`EuiButtonGroupButton` to `450`
([#8734](elastic/eui#8734))
- Added color pallete functions and related hooks:
([#8725](elastic/eui#8725))
  - `euiPaletteSkyBlue`, `useEuiPaletteSkyBlue`
  - `euiPaletteYellow`, `useEuiPaletteYellow`
  - `euiPaletteOrange`, `useEuiPaletteOrange`
- Added new tokens on `colors.vis`:
([#8725](elastic/eui#8725))
  -  `euiColorVisNeutral0`
  -  `euiColorVisNeutral1`
  -  `euiColorVisWarning1`
  -  `euiColorVisRisk0`
  -  `euiColorVisRisk1`
- Updated the value of token `colors.vis.euiColorVisWarning0`
([#8725](elastic/eui#8725))
- Updated EuiFilterButton's `numActiveFilters` prop to accept percentage
values ([#8705](elastic/eui#8705))

**Bug fixes**

- Fixed visual positioning issue for notifications in
`EuiHeaderSectionItemButton`
([#8736](elastic/eui#8736))
- Fixed a visual issue where `EuiCollabsibleNavItem` did not have a
visible selected state
([#8736](elastic/eui#8736))
- Fixed handling of unregistered code block languages in
`EuiMarkdownFormat` ([#8729](elastic/eui#8729))

**Breaking changes**

- Renamed `colors.vis.euiColorVisNeutral0` to `euiColorVisBase0`
([#8725](elastic/eui#8725))

### `@elastic/eui-theme-borealis`

##
[`v2.0.0`](https://github.com/elastic/eui/blob/main/packages/eui-theme-borealis/changelogs/CHANGELOG_2025.md#v200)

- Added new tokens on `colors.vis`:
([#8725](elastic/eui#8725))
  -  `euiColorVisNeutral0`
  -  `euiColorVisNeutral1`
  -  `euiColorVisWarning1`
  -  `euiColorVisRisk0`
  -  `euiColorVisRisk1`
- Updated the value of token `colors.vis.euiColorVisWarning0`
([#8725](elastic/eui#8725))

**Bug fixes**

- Fixed missing source map warnings emitted by some bundlers by
excluding source maps from being published
([#8764](elastic/eui#8764))
- To align with `@elastic/eui` and many other popular packages, we made
a call to not ship source maps anymore

**Breaking changes**

- Renamed `colors.vis.euiColorVisNeutral0` to `euiColorVisBase0`
([#8725](elastic/eui#8725))

<!--ONMERGE {"backportTargets":["8.19","9.0"]} ONMERGE-->

---------

Co-authored-by: Lene Gadewoll <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mgadewoll mgadewoll mgadewoll left review comments

@weronikaolejniczak weronikaolejniczak weronikaolejniczak approved these changes

Assignees

@tkajtoch tkajtoch

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[EuiFilterButton] Allow percentage values in numActiveFilters prop

4 participants

@tkajtoch @elasticmachine @weronikaolejniczak @mgadewoll