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

Enable setting styles for filter button as a configuration. #2182

Open
wants to merge 3 commits into
base: development
Choose a base branch
from

Conversation

HussamAlhennawi
Copy link

@HussamAlhennawi HussamAlhennawi commented Jan 19, 2025

Hi,

This PR represents an implementation to enable setting the styles for filter button and filter button badge as a configuration through the following functions:

  • setFilterButtonAttributes(...)
  • setFilterButtonBadgeAttributes(...)

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

New Feature Submissions:

  1. Does your submission pass tests and did you add any new tests needed for your feature?
  2. Did you update all templates (if applicable)?
  3. Did you add the relevant documentation (if applicable)?
  4. Did you test locally to make sure your feature works as intended?

Copy link

codecov bot commented Jan 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.00%. Comparing base (e58bfba) to head (e07bcc1).

Additional details and impacted files
@@                Coverage Diff                @@
##             development    #2182      +/-   ##
=================================================
+ Coverage          90.98%   91.00%   +0.01%     
- Complexity          2011     2015       +4     
=================================================
  Files                218      220       +2     
  Lines               4539     4549      +10     
=================================================
+ Hits                4130     4140      +10     
  Misses               409      409              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lrljoe
Copy link
Collaborator

lrljoe commented Jan 19, 2025

Please add in the tests, as otherwise it'll queue until I have time to add them in!

@lrljoe
Copy link
Collaborator

lrljoe commented Jan 19, 2025

Also noting that there's a significant change pending relating to Filter trait namespaces, so I may need to adjust this PR pretty significantly to get it in line with the pending change.

@HussamAlhennawi
Copy link
Author

Thanks, I will work on tests and change the required namespaces depending on #2181 ASAP (3 days at most).

@HussamAlhennawi
Copy link
Author

Tests for (filter button and filter button badge styling) were added, Please feel free to adjust Filter trait namespaces to align with the pending change.

@lrljoe
Copy link
Collaborator

lrljoe commented Jan 24, 2025

What I'll do, is merge in the other PR which covers off all of the new namespaces etc, then I'll do some tweaks on this one to merge it in.

You may notice that the other PR also adjusts your other attribute methods to use the centralised approach. I'm trying to migrate any new attributes etc to this approach, as it should make keeping everything standardised a little easier.

At the moment, there's a mix of "default" being true/false, so I'm trying to use some core methods so that it is standard (For anything new at least)

Great work btw on all of the new attributes, and really appreciated.

@lrljoe
Copy link
Collaborator

lrljoe commented Feb 9, 2025

So with all of the recent underpinning work, this has a few conflicts.

Once everything else is fully tested/released, then I'll come back to this and look at differences and merge appropriate changes in.

@HussamAlhennawi
Copy link
Author

Perfect, Thanks for the updates.

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.

2 participants