Skip to content

Conversation

@yihao03
Copy link
Contributor

@yihao03 yihao03 commented Feb 5, 2026

  • Add tag counts to cardstack
  • Add tests for tag counts

What is the purpose of this pull request?

  • Documentation update
  • Bug fix
  • Feature addition or enhancement
  • Code maintenance
  • DevOps
  • Improve developer experience
  • Others, please explain:

Overview of changes:
Add card counts to tag. Updated the updateTagMapping() function in CardStack.vue to calculate
the number of cards associated with each tag and store this information in the tagMapping.

image

Displays the count between the tag text and the tickbox in the tags. Closes #2808

Anything you'd like to highlight/discuss:
How should I make it look prettier or is this fine?

Testing instructions:
Run markbind serve -d to serve the docs and checkout the cardstack documentation pages.

Proposed commit message: (wrap lines at 72 characters)
Add card counts to tags in CardStack component


Checklist: ☑️

  • Updated the documentation for feature additions and enhancements
  • Added tests for bug fixes or features
  • Linked all related issues
  • No unrelated changes

Reviewer checklist:

Indicate the SEMVER impact of the PR:

  • Major (when you make incompatible API changes)
  • Minor (when you add functionality in a backward compatible manner)
  • Patch (when you make backward compatible bug fixes)

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):

  • To be included in the release note for any feature that is made obsolete/breaking

Give a brief explanation note about:

  • what was the old feature that was made obsolete
  • any replacement feature (if any), and
  • how the author should modify his website to migrate from the old feature to the replacement feature (if possible).

@yihao03
Copy link
Contributor Author

yihao03 commented Feb 5, 2026

image

this is how it looks like now

@codecov
Copy link

codecov bot commented Feb 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.09%. Comparing base (fe1bcba) to head (c64f963).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2828      +/-   ##
==========================================
- Coverage   72.20%   72.09%   -0.11%     
==========================================
  Files         134      134              
  Lines        7461     7411      -50     
  Branches     1648     1645       -3     
==========================================
- Hits         5387     5343      -44     
+ Misses       1946     1940       -6     
  Partials      128      128              

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@yihao03 yihao03 requested review from Incogdino, Copilot and gerteck and removed request for Copilot and gerteck February 5, 2026 02:43
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds card count indicators to tags in the CardStack component to help users understand how many cards are associated with each tag. The implementation modifies the tag mapping logic to track counts and displays them in badges alongside the existing tag selection checkmarks.

Changes:

  • Modified updateTagMapping() function to calculate and store the number of cards associated with each tag
  • Added a count badge display in the tag UI, positioned between the tag name and the selection checkmark
  • Added comprehensive test coverage for the tag counting feature including edge cases

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
packages/vue-components/src/cardstack/CardStack.vue Added count property to tag mappings, updated initialization logic to track tag occurrences, added count badge UI element, and adjusted CSS spacing for tag indicators
packages/vue-components/src/tests/snapshots/CardStack.spec.js.snap Updated snapshot to reflect the new count badge in tag display
packages/vue-components/src/tests/CardStack.spec.js Added four new test cases covering count initialization for custom tags, count increments for duplicate tags, count badge display, and badge ordering

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@yihao03 yihao03 self-assigned this Feb 5, 2026
@gerteck
Copy link
Member

gerteck commented Feb 5, 2026

Can consider adding a boolean flag to set as false or true to disable the count per tag (default: show)

Copy link
Member

@gerteck gerteck left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@damithc
Copy link
Contributor

damithc commented Feb 6, 2026

@yihao03 Also update the PR description with the latest UI screenshots.

@gerteck gerteck marked this pull request as draft February 10, 2026 02:58
@MoshiMoshiMochi
Copy link
Contributor

Hi @yihao03, great work on this feature! As we discussed in class, perhaps it would be more intuitive for the user to have the count be reactive and update based on the cards currently displayed in the cardstack.

Move components from data() to setup() and use reactive object to make
use of Vue 3 reactivity=
@yihao03
Copy link
Contributor Author

yihao03 commented Feb 10, 2026

good idea @MoshiMoshiMochi ! It has been implemented

@yihao03 yihao03 marked this pull request as ready for review February 10, 2026 09:01
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 183 to 189
tagCounts() {
const counts = {};
this.cardStackRef.children.forEach((child) => {
if (child.disabled) return;
const searchTerms = this.cardStackRef.searchTerms || [];
const searchTarget = (child.computeTags.join(' ')
+ child.keywords + child.headerText).toLowerCase();
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

tagCounts builds counts as a plain object with user-controlled tag names as keys. Tags like __proto__ / constructor can cause prototype pollution or unexpected lookups, and falsy inherited keys can affect tagCounts[key] || 0. Use a Map or Object.create(null) for counts (and adjust the template lookup accordingly) to avoid prototype inheritance issues.

Copilot uses AI. Check for mistakes.
Copy link
Member

@gerteck gerteck Feb 10, 2026

Choose a reason for hiding this comment

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

  • In JavaScript, plain objects {} inherit properties from the global Object.prototype.
  • Bug: If a user-provided tag is named "toString" or "hasOwnProperty", your code counts[tag] = (counts[tag] || 0) + 1 will fail. It will try to add 1 to a function, resulting in NaN.

Means if use {} directly, using tagnames like valueOf constructor etc will break, use a map instead

const counts = new Map(); 

and also change the methods to .get() and .set()

image

@gerteck
Copy link
Member

gerteck commented Feb 11, 2026

image

LGTM

@gerteck gerteck merged commit 01b8ce4 into MarkBind:master Feb 11, 2026
9 of 10 checks passed
@github-actions github-actions bot added the r.Minor Version resolver: increment by 0.1.0 label Feb 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c.Feature 🚀 r.Minor Version resolver: increment by 0.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Card Stack: Add count for each tag

4 participants