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

Allow disabling built-in CSS classes for FilterSearch component #482

Merged
merged 5 commits into from
Jan 10, 2025

Conversation

k-gerner
Copy link
Contributor

Add a new flag to FilterSearchProps named disableBuiltInClasses.

To address #481
If this property is true, and there are customCssClasses set, then
we will completely remove the builtin CSS classes from the
component.

TEST=manual
Spun up test site and set the property on a component. Saw the
classes get replaced as expected.

To address #481
Adds a new flag to FilterSearchProps named disableBuiltInClasses.
If this property is true, and there are customCssClasses set, then
we will completely remove the builtin CSS classes from the
component.

TEST=manual
Spun up test site and set the property on a component. Saw the
classes get replaced as expected.
@k-gerner k-gerner requested a review from a team as a code owner January 10, 2025 20:55
Copy link
Contributor

Current unit coverage is 92.08312716476992%
Current visual coverage is 79.01234567901234%
Current combined coverage is 92.58160237388724%

@coveralls
Copy link

coveralls commented Jan 10, 2025

Coverage Status

coverage: 85.327% (-0.05%) from 85.38%
when pulling 708b22e on dev/filter-search-disable-builtin-classes
into 109b443 on main.

Copy link
Contributor

@Fondryext Fondryext left a comment

Choose a reason for hiding this comment

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

Don't forget to increment the version. Maybe a new minor version? Could be argued this is kinda like a bugfix though, so I could be fine with a patch. I see an older pull request with Max that hasn't been shipped. May be worth syncing with him real quick to see what the state of that is.

@k-gerner
Copy link
Contributor Author

I was thinking that I'd just roll up Max's change at the same time and keep the v1.6.7 since that change hadn't been shipped yet, but I think you're actually right that it's more of a minor version change here. I'll update the version to v1.7.0.

(The parent branch of this one will have the wrong name though but that's fine)

@k-gerner k-gerner merged commit 6a7ae5a into main Jan 10, 2025
21 of 22 checks passed
@k-gerner k-gerner deleted the dev/filter-search-disable-builtin-classes branch January 10, 2025 21:25
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