-
Notifications
You must be signed in to change notification settings - Fork 140
[card-stack] Feature match all selected toggle button #2713
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
Conversation
This reverts commit 5b25e74.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2713 +/- ##
==========================================
+ Coverage 52.84% 52.88% +0.03%
==========================================
Files 130 130
Lines 7162 7169 +7
Branches 1503 1503
==========================================
+ Hits 3785 3791 +6
- Misses 3072 3223 +151
+ Partials 305 155 -150 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Incogdino
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! This feature defitnitely gives users greater flexibility when choosing between the different search options.
Left some comments mainly targetting documentation and naming.
| </span> | ||
| </span> | ||
| <span class="toggle-badge" @click="toggleFilterMode"> | ||
| match all selected |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Match all selected - Could you capitalize the start of the word?
| }, | ||
| methods: { | ||
| toggleFilterMode() { | ||
| this.filterMode = this.filterMode === 'AND' ? 'OR' : 'AND'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps using a Boolean to represent this would be better? Changing the naming of filterMode to isAndFilter and toggling between them with the Logical NOT operator !
| - a `card` can be given a `header` attribute (optional). | ||
| - tags can be added to cards using the `tag` attribute, which can then be used to filter cards. | ||
| - the `searchable` attribute can be used to make the Card Stack searchable based on tags and headers. | ||
| - The `match all selected` toggle filters cards based on an AND condition across the selected tags. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a minor casing issue
"The Match all selected toggle filters cards based on an AND condition across the selected tags."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also can maybe add a short sentence behind talking about the default OR behaviour if not selected.
| :class="['btn', 'btn-sm', `tag-badge`]" | ||
| > | ||
| <span class="badge bg-light text-dark tag-indicator"> | ||
| <span v-if="filterMode === 'AND'">✓</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here. Can use Boolean naming?
What is the purpose of this pull request?
close #2686
Overview of changes:
Add a button to improve UX by allowing readers to choose and/or filtering when multiple tags are selected
Anything you'd like to highlight/discuss:
Testing instructions:
Check if filtering works well when "match all selected" is set to true/false with multiple tags checked
Screencast from 2025년 04월 23일 23시 25분 18초.webm
Proposed commit message: (wrap lines at 72 characters)
Feat: Add multi-tag filtering options
Adds a button or toggle to allow users to switch between
"match all selected tags" and "match any selected tags" when
filtering content. This improves the user experience for readers
navigating items with multiple tags applied.
The filtering logic is updated to support both modes based on
the user's selection. Documentation is updated accordingly.
close #2686
Checklist: ☑️
Reviewer checklist:
Indicate the SEMVER impact of the PR:
At the end of the review, please label the PR with the appropriate label:
r.Major,r.Minor,r.Patch.Breaking change release note preparation (if applicable):