Skip to content

Conversation

@yihao03
Copy link

@yihao03 yihao03 commented Jan 29, 2026

  • feat: add custom tag order and colors for CardStack component
  • Refactor and abstract code

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:
Implemented custom tag ordering and color in the CardStack component as mentioned in issue #2783.
Users are now able to include:

<tags>
  <tag name="TagName" color="#RRGGBB" />
  <tag name="TagName" color="<bootstrap_colors>" />
</tags>

to their cardstack. Note that users are not required to include all tags in the custom order; any tags not specified will be appended in default order.
Colors can be specified in hex format or bootstrap color names.

Anything you'd like to highlight/discuss:

Testing instructions:

  1. Pull the branch and run the site locally using markbind serve -d
  2. Navigate to the CardStack component page and verify that the tags appear in the specified custom order with the correct colors.
  3. Optionally, add new cards with tags to ensure that unspecified tags are appended in default order.

Proposed commit message: (wrap lines at 72 characters)
Enable custom tag order and colors 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).

- Add <tags> syntax parsing in MdAttributeRenderer to extract tag configurations
- Implement processCardStackAttributes() to parse <tag> elements with name and color
- Add tagConfigs and dataTagConfigs props to CardStack component
- Implement tag ordering algorithm that respects custom config order
- Add color normalization supporting both hex colors and Bootstrap color names
- Update badge rendering with conditional styling (Bootstrap classes vs inline styles)
- Add isBootstrapColor() and getTextColor() helper methods for color handling
- Apply same color logic to Card component for visual consistency
- Add comprehensive test coverage for custom tag ordering and colors
- Update documentation with examples for hex colors and Bootstrap color names
- Document new <tags> and <tag> element syntax and options
@codecov
Copy link

codecov bot commented Jan 29, 2026

Codecov Report

❌ Patch coverage is 90.00000% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.18%. Comparing base (58f396f) to head (ca2b4f3).

Files with missing lines Patch % Lines
packages/core/src/html/NodeProcessor.ts 25.00% 3 Missing ⚠️
packages/core/src/html/codeblockProcessor.ts 33.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2809      +/-   ##
==========================================
+ Coverage   71.85%   72.18%   +0.33%     
==========================================
  Files         134      134              
  Lines        7340     7449     +109     
  Branches     1563     1622      +59     
==========================================
+ Hits         5274     5377     +103     
- Misses       2020     2026       +6     
  Partials       46       46              

☔ 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.

@damithc
Copy link
Contributor

damithc commented Jan 29, 2026

Note that users are not required to include all tags in the custom order; any tags not specified will be appended in default order.

@yihao03 those 'unlisted' tags appear in default order but after the listed tags? Or they appear mixed together with 'listed' tags?

@yihao03
Copy link
Author

yihao03 commented Jan 29, 2026

@damithc they will appear after the specified tags

@yihao03 yihao03 requested review from Harjun751, MoshiMoshiMochi and gerteck and removed request for gerteck January 30, 2026 02:18
@yihao03 yihao03 self-assigned this Jan 30, 2026
Copy link
Contributor

@Harjun751 Harjun751 left a comment

Choose a reason for hiding this comment

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

Commenting on some minor code quality nits here. Code reads good!


The example above also illustrates how to use the `keywords` attribute to specify additional search terms for a card.

### Custom Tag Order and Colors
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor nit: I wonder if it'd be better (as a reader) for the custom tag order to be above the MRQ cardstack example?

I think I'd prefer it that way but I realize that it might be subjective so if you think this is better/ok that's fine!

Copy link
Contributor

Choose a reason for hiding this comment

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

Wow, didn't imagine it'd be this hard to get the colors like that. Cool solution.


You can customize the order and colors of tags by using a `<tags>` element inside the `cardstack`:

<include src="codeAndOutputSeparate.md" boilerplate >
Copy link
Member

Choose a reason for hiding this comment

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

Maybe you could consider using `src="codeAndOutput.md"!

<include src="codeAndOutput.md" boilerplate>
<variable name="highlightStyle">html</variable>
<variable name="code">

Only need to include once if the code is the same

</variable>
</include>

You could also replace the top example (the existing docs example) with the following as well to incoporate better reuse:

<include src="codeAndOutput.md" boilerplate >
<variable name="highlightStyle">html</variable>
<variable name="code">
<cardstack searchable>
  <card header="**Winston Churchill**" tag="Success, Perseverance">
    Success is not final, failure is not fatal: it is the courage to continue that counts
  </card>
  <card header="**Albert Einstein**" tag="Success, Perseverance">
    In the middle of every difficulty lies opportunity
  </card>
  <card header="**Theodore Roosevelt**" tag="Motivation, Hard Work">
    Do what you can, with what you have, where you are
  </card>
  <card header="**Steve Jobs**" tag="Happiness, Mindset">
    Your time is limited, so don’t waste it living someone else’s life
  </card>
</cardstack>
</span>
</variable>

Copy link
Author

Choose a reason for hiding this comment

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

interesting! when would codeAndOutputSeparate.md be useful then?

Copy link
Member

Choose a reason for hiding this comment

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

It's useful for the question quiz example for instance

in the code block, we can show <!-- Details of questions omitted. --> and not have to show the entire code block of the questions.

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 support for custom tag ordering and colors in the CardStack component, wired end‑to‑end from Markdown syntax through the core HTML processor to the Vue UI, with supporting utility functions and documentation.

Changes:

  • Introduced shared color utilities (BADGE_COLOURS, isBootstrapColor, getTextColor, normalizeColor, MIN_TAGS_FOR_SELECT_ALL) and HTML escape helpers for both core and Vue packages.
  • Extended CardStack and Card Vue components to consume serialized tag configuration metadata (data-tag-configs / tagConfigs) and render tags in custom order with either Bootstrap or hex colors.
  • Updated the core Markdown/HTML processing pipeline (MdAttributeRenderer, NodeProcessor) to parse <tags><tag .../></tags> blocks in cardstack, serialize them into a safely escaped data-tag-configs attribute, and documented the feature with examples and tests.

Reviewed changes

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

Show a summary per file
File Description
packages/vue-components/src/utils/utils.js Adds escapeHTML/unescapeHTML helpers for safely handling serialized JSON in attributes on the Vue side, mirroring the core escape behavior.
packages/vue-components/src/utils/colors.js Introduces reusable badge color utilities (Bootstrap mapping, contrast-aware text color, normalization) used by CardStack/Card.
packages/vue-components/src/cardstack/CardStack.vue Refactors tag color constants into shared utilities, adds tagConfigs/dataTagConfigs props, parses and applies custom tag order/colors to tagMapping, and adjusts rendering to support both Bootstrap and hex colors.
packages/vue-components/src/cardstack/Card.vue Updates tag badge rendering to use the new color utilities so individual cards reflect the configured tag colors consistently.
packages/vue-components/src/__tests__/utils.spec.js Adds unit tests for the Vue escapeHTML/unescapeHTML functions, including JSON and entity round‑trip behavior.
packages/vue-components/src/__tests__/colors.spec.js Covers BADGE_COLOURS, MIN_TAGS_FOR_SELECT_ALL, isBootstrapColor, getTextColor, and normalizeColor to ensure color handling stays consistent.
packages/vue-components/src/__tests__/CardStack.spec.js Extends CardStack tests to verify custom tag ordering, hex and Bootstrap color application, fallback behavior for unconfigured tags, and robustness to invalid tag configs.
packages/core/test/unit/utils/escape.test.ts Adds tests for the core escapeHTML implementation ensuring correct escaping of special characters and entities.
packages/core/test/unit/html/MdAttributeRenderer.test.ts Introduces tests around processCardStackAttributes for parsing <tags> blocks, building data-tag-configs, escaping values correctly, and removing the <tags> node.
packages/core/src/utils/escape.ts Implements the core escapeHTML function used when serializing tag configuration JSON into HTML-safe attributes.
packages/core/src/html/NodeProcessor.ts Hooks cardstack nodes into the attribute-processing pipeline by delegating to MdAttributeRenderer.processCardStackAttributes.
packages/core/src/html/MdAttributeRenderer.ts Adds processCardStackAttributes to extract <tag> definitions under <tags>, build a typed config array, escape it, and set data-tag-configs on the cardstack node.
docs/userGuide/syntax/cardstacks.md Documents the new <tags>/<tag> syntax, explains ordering and color options (hex and Bootstrap names), and adds illustrative examples with rendered output.

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

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.

Thanks @yihao03 for this PR, great work! Added some comments

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 13 out of 13 changed files in this pull request and generated 3 comments.


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

Comment on lines +14 to +21
export function isBootstrapColor(color) {
return BADGE_COLOURS.some(c => c === color);
}

export function getTextColor(backgroundColor) {
if (!backgroundColor || backgroundColor.startsWith('bg-')) {
return '#000';
}
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

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

isBootstrapColor only returns true for values that exactly match entries in BADGE_COLOURS, which means color values that start with bg- but are not in that fixed list (e.g. bg-warning or custom bg-custom) are treated as non‑Bootstrap colors and end up being applied via backgroundColor inline style, which produces invalid CSS and incorrect rendering. To keep behaviour consistent with normalizeColor (which explicitly treats any bg- value as a Bootstrap class) and avoid this bug, consider changing isBootstrapColor to detect Bootstrap classes more generically (for example by checking color.startsWith('bg-')), rather than restricting it to the fixed palette.

Copilot uses AI. Check for mistakes.
Comment on lines 235 to 249
export function escapeHTML(htmlStr) {
return String(htmlStr)
.replace(/&(?!\w+;)/g, '&amp;')
.replace(/</g, '&lt;')
.replace(/>/g, '&gt;')
.replace(/"/g, '&quot;');
}

return {style: mainStyle, icon: iconStyle};
export function unescapeHTML(htmlStr) {
return String(htmlStr)
.replace(/&quot;/g, '"')
.replace(/&gt;/g, '>')
.replace(/&lt;/g, '<')
.replace(/&amp;/g, '&');
}
Copy link

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

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

There are now two independent implementations of HTML escaping logic: escapeHTML/unescapeHTML in packages/vue-components/src/utils/utils.js and escapeHTML in packages/core/src/utils/escape.ts, which must remain behaviourally identical for cardstack tag configs to round‑trip correctly between server and client. To reduce the risk of future divergence and subtle bugs, consider centralising this logic (or at least adding a shared test/contract) so that both code paths share the same implementation or are clearly validated against each other.

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

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

Do you want to consider using

This way, we just have to make sure that the package is at the same version for both vue component and core package.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.

logic lgtm, just some file structure changes for clarity of responsibilties

}

// eslint-disable-next-line class-methods-use-this
processCardStackAttributes(node: MbNode) {
Copy link
Member

Choose a reason for hiding this comment

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

Consider moving this method into its own class or file (e.g. cardStackProcessor.ts in /html, since it does not do any markdown rendering as in MdAttributeRenderer.

The current structure seems to suggest processCardStackAttributes processes markdown, while in actuality it is manipulating the HTML Dom and injecting attributes independent of markdown syntax.

For e.g. headerProcessor, footnoteProcessor are either their own class or export their own functions.

Otherwise, the logic for the function lgtm

…ub.com:yihao03/markbind into feat/cardstack-tag-order-colors-implementation
this.processSlotAttribute(node, 'header', true);
}

// eslint-disable-next-line class-methods-use-this
Copy link
Member

Choose a reason for hiding this comment

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

leftover code

has,
};

export type CardStackTagConfig = { name: string; color?: string };
Copy link
Member

Choose a reason for hiding this comment

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

^

isInline: boolean, slotName = attribute): void {
const hasAttributeSlot = node.children
&& node.children.some(child => getVslotShorthandName(child) === slotName);
&& node.children.some(child => getVslotShorthandName(child) === slotName);
Copy link
Member

Choose a reason for hiding this comment

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

^ can revert this so it does not show up in the pr changes

@@ -0,0 +1,55 @@
import { encode } from 'html-entities';
import { MbNode, NodeOrText } from '../utils/node';
import { CardStackTagConfig } from './MdAttributeRenderer';
Copy link
Member

Choose a reason for hiding this comment

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

should this be defined and exported here?

Copy link
Member

Choose a reason for hiding this comment

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

should this file be renamed

// @params (function, delay_prop or value, default_value)
export function delayer (fn, varTimer, ifNaN = 100) {
function toInt (el) { return /^[0-9]+$/.test(el) ? Number(el) || 1 : null }
export function delayer(fn, varTimer, ifNaN = 100) {
Copy link
Member

Choose a reason for hiding this comment

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

try to minimize whitespace changes in PRs because it adds noise to the PR file changes,

for instance, i had to look through all whitespace changes to find that there were no functional changes in this file

Copy link
Member

Choose a reason for hiding this comment

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

Linting PRs should be a separate PR with only whitespace changes, for instance

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants