-
Notifications
You must be signed in to change notification settings - Fork 134
Add card stack component #2648
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
Add card stack component #2648
Conversation
The CardStack component contains cards that can be specified using Markbind's custom components. The CardStack component comes contains a search feature to filter through the cards identified using tags and keywords.
Tag colouring help to provide better visual appeal to the tags. Due to the use of bootstrap badge for tags, the choice of color is limited to 8 different colors. If there are more than 8 tags, the color will recycle, making the tag coloration non unique. In future, we can consider 1. adding more color schemes. 2. making the tags clickable as a button to filter search cards.
Previously, faces the following issues: 1. Does not allow header to be part of the search fields. 2. Collects search information prior to each search input. The search feature now allows headers to be part of the search field and preprocesses the search data on mount.
Previous version of card stack does not allow users to search by tag. Changes added: 1. Add search by tags (click activated) 2. Update snapshots
1. Added handling for erroneous block values (fallback to closet predefined values). 2. Update format to include space when displaying keywords. 3. Update supported block values for cardstack in docs.
This looks like a really cool and useful feature! Play around with it here: |
There is some weird behavior with the searchbar of the card stack. It seems to only search using the last word in the search bar. For example, consider this sequence of searches:
The expected behavior depends on whether the searchbar should be an OR or an AND search but either way, the actual behavior doesn't seem to follow either. The actual behavior seems to be that the searchbar only uses the last word, so for 1, 2 and 3, it searches "basic", "card" and "question" respectively. Is this intended behavior? Screenshots below: |
In terms of the tags, the current implementation seems to be an OR implementation; I am thinking if AND is more intuitive. Perhaps we could also provide a way to choose between the two behaviors? And as a nit, in terms of the design, adding some space between the "Tags:" and the actual tags would be nice. That aside, as @AgentHagu pointed out, the behavior of the search bar is rather unintuitive. Filtering based on the last keyword seems to be the current behavior, which is rather unexpected. E.g. this would not be the expected result for users, I would imagine: @Incogdino What are your thoughts? |
Search feature now searches for all matches with the specified value instead of just the last word.
…ogdino/markbind into branch-addCardStackComponent
I discussed this with prof @damithc the other day during lecture and the consensus was that an OR search would better fit the purpose here since users are most likely going to filter according to what they want to appear in the card stack. As for whether we should allow users to choose between AND/ OR tag search, it is a good idea and I think we should have it too but it might lead to some inconsistencies in the final site when multiple card stacks are involved in the application and users toggle between them.
Will work on this!
I have fixed this issue in the latest commit. |
- Removed overly complex documentation in favor of simpler and more realistic use case of the card stack component. - Added more details into the description of the component.
1. Updated tags to flow with the search bar and overflow under it. 2. Updated snapshots.
Key changes made: - Removal of $children -> changed to provide/inject
I see that the latest commit is updating this feature to Vue 3 - is the PR ready for review? |
Let me update the docs to include the icons that markbind supports (for #2553) |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2648 +/- ##
==========================================
- Coverage 52.16% 51.90% -0.27%
==========================================
Files 128 130 +2
Lines 6999 7104 +105
Branches 1546 1570 +24
==========================================
+ Hits 3651 3687 +36
- Misses 3049 3118 +69
Partials 299 299 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Could we review this pr for now and leave #2553 on hold in the meantime. I am trying to find a way to retireve all the icon names without having to manually copy and paste them from the source itself. If anyone has any ideas, please do let me know! |
I see, so we possibly merge the feature in first and leave the actual implementation on the MarkBind documentation to another PR later on. Sure, let's do that and keep this PR to introducing the new feature only. |
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.
LGTM, the feature works well; I couldn't find any regressions. Features like selecting between AND / OR, as well as using this for icons, can be followed-up in later PRs. Thanks @Incogdino!
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.
Looks good to me too!
@lhw-1 Each PR must have a SEMVER impact label, please remember to label the PR properly. |
What is the purpose of this pull request?
Closes #2553
Overview of changes:
Add CardStack Component
Anything you'd like to highlight/discuss:
Testing instructions:
Proposed commit message: (wrap lines at 72 characters)
Add CardStack component
Features:
Current limitations:
markbind's badge which inherently uses bootstrap's badge.
Since bootstrap grid is used, the number of columns allowed is
limited to 5 preset values.
Left to do:
Future improvements:
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):