feat(components): dlt-3096 add slot classes#1214
Merged
Ignacio Ropolo (iropolo) merged 5 commits intonextfrom Apr 21, 2026
Merged
feat(components): dlt-3096 add slot classes#1214Ignacio Ropolo (iropolo) merged 5 commits intonextfrom
Ignacio Ropolo (iropolo) merged 5 commits intonextfrom
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Adds configurable slot-class props across many Vue components (modal, badge, button, input, chip, popover, tooltip, empty_state, toast, notice, banner, hovercard, dropdown, combobox variants), broadens item_layout prop types, and adds tabsClass with deprecated tabListClass fallback. Overall Judgement: ✅ Ready to merge — Non-breaking, well-scoped API additions that pass through class props, widen prop types, and include clear deprecation handling with minimal runtime risk. WalkthroughAdds many new Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Ignacio Ropolo (iropolo)
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
packages/dialtone-vue/components/button/button.vue (1)
354-356: Clarify deprecatediconClassreplacement guidance.Since block icon slots are also supported, consider updating the deprecation note to include
blockStartIconClassandblockEndIconClassas valid replacements where applicable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/dialtone-vue/components/button/button.vue` around lines 354 - 356, The deprecation note for iconClass is incomplete: update its JSDoc/comment to mention that consumers should use startIconClass or endIconClass for inline icons and blockStartIconClass or blockEndIconClass for block icon slots; reference the existing identifiers iconClass, startIconClass, endIconClass, blockStartIconClass, and blockEndIconClass so maintainers can find and update the comment in button.vue accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/dialtone-vue/components/chip/chip.vue`:
- Line 25: Restore the component's base avatar class in the wrapper's class
binding instead of replacing it with only avatarClass: ensure the wrapper
element(s) that currently use ":class=\"avatarClass\"" keep the static base
class (e.g., "dt-chip__avatar" or the component's base avatar class) and then
append avatarClass (e.g., :class="['dt-chip__avatar', avatarClass]"); update
both occurrences that bind avatarClass (the one near the top using avatarClass
and the second occurrence around the 166-172 block) so default chip avatar
styling is preserved while still allowing custom classes.
In `@packages/dialtone-vue/components/empty_state/empty_state.vue`:
- Around line 111-133: Add storybook argTypes and tests for the three new props
illustrationClass, iconClass, and bodyClass: follow the avatar component's
argTypesData pattern (e.g., avatarClass, overlayClass) and add entries for
illustrationClass, iconClass, and bodyClass in empty_state.stories.js so they
appear in controls/documentation; then extend empty_state.test.js to include
unit tests asserting these props are passed through and applied to the correct
wrapper elements (use the component's prop names illustrationClass, iconClass,
bodyClass and query the illustration/icon/body wrapper selectors used in the
component to verify classes are rendered).
In `@packages/dialtone-vue/components/modal/modal.vue`:
- Line 54: The headerClass prop is not applied when rendering the default title
path in the Modal component; update the default header/title render branches
(where dt-text is used) to include the headerClass in the class binding so the
prop takes effect. Specifically, modify the dt-text render in modal.vue (the
default title-render path) to merge the existing "d-modal__header" class with
headerClass (same change also needed in the other similar title/header render
blocks noted around the other occurrences referenced), ensuring the :class
binding includes headerClass rather than hardcoding only "d-modal__header".
In `@packages/dialtone-vue/components/popover/popover.vue`:
- Around line 451-457: Add the new anchorClass prop documentation and usage
everywhere it must be kept in sync: update the Popover component docs JSON entry
to include the anchorClass prop (type, default, description), add/modify the
Storybook story for Popover to demonstrate anchorClass with string/array/object
examples, add unit tests that assert the anchor wrapper receives the passed
class variations (referencing the anchorClass prop on the Popover component),
update the VuePress component docs page to show the prop and usage example, and
ensure the MCP server data/schema contains the anchorClass metadata so all six
artifacts (tests, story, docs JSON, VuePress, MCP server, and the component)
remain synchronized.
In `@packages/dialtone-vue/components/toast/layouts/toast_layout_alternate.vue`:
- Around line 155-170: The alternate layout is missing the actionClass prop and
therefore drops the parent binding (inheritAttrs: false); add a prop declaration
named actionClass with the same type/default as iconClass/titleClass (type:
[String, Array, Object], default: '') in the component props, then pass it into
the action component by binding it on the dt-notice-action element (e.g.
:action-class="actionClass" or v-bind:action-class="actionClass") so the parent
DtToast's action-class is forwarded.
---
Nitpick comments:
In `@packages/dialtone-vue/components/button/button.vue`:
- Around line 354-356: The deprecation note for iconClass is incomplete: update
its JSDoc/comment to mention that consumers should use startIconClass or
endIconClass for inline icons and blockStartIconClass or blockEndIconClass for
block icon slots; reference the existing identifiers iconClass, startIconClass,
endIconClass, blockStartIconClass, and blockEndIconClass so maintainers can find
and update the comment in button.vue accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 9eca20d0-1314-44b8-a267-9436f649da52
📒 Files selected for processing (23)
packages/dialtone-vue/components/badge/badge.vuepackages/dialtone-vue/components/banner/banner.vuepackages/dialtone-vue/components/button/button.vuepackages/dialtone-vue/components/chip/chip.vuepackages/dialtone-vue/components/combobox_multi_select/combobox_multi_select.vuepackages/dialtone-vue/components/combobox_with_popover/combobox_with_popover.vuepackages/dialtone-vue/components/dropdown/dropdown.vuepackages/dialtone-vue/components/empty_state/empty_state.vuepackages/dialtone-vue/components/hovercard/hovercard.vuepackages/dialtone-vue/components/input/input.vuepackages/dialtone-vue/components/item_layout/item_layout.vuepackages/dialtone-vue/components/modal/modal.vuepackages/dialtone-vue/components/notice/notice.vuepackages/dialtone-vue/components/notice/notice_action.vuepackages/dialtone-vue/components/notice/notice_content.vuepackages/dialtone-vue/components/notice/notice_icon.vuepackages/dialtone-vue/components/popover/popover.vuepackages/dialtone-vue/components/tab/tab_group.vuepackages/dialtone-vue/components/toast/layouts/toast_layout_alternate.vuepackages/dialtone-vue/components/toast/layouts/toast_layout_alternate_icon.vuepackages/dialtone-vue/components/toast/layouts/toast_layout_default.vuepackages/dialtone-vue/components/toast/toast.vuepackages/dialtone-vue/components/tooltip/tooltip.vue
| </span> | ||
| <span | ||
| v-else-if="hasSlotContent($slots.avatar)" | ||
| :class="avatarClass" |
Contributor
There was a problem hiding this comment.
Restore the base avatar class in the wrapper binding.
Line 25 drops the component’s base avatar class and can break default chip avatar styling. Keep the base class and append avatarClass.
🛠️ Proposed fix
- :class="avatarClass"
+ :class="['d-chip__avatar', avatarClass]"Also applies to: 166-172
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/dialtone-vue/components/chip/chip.vue` at line 25, Restore the
component's base avatar class in the wrapper's class binding instead of
replacing it with only avatarClass: ensure the wrapper element(s) that currently
use ":class=\"avatarClass\"" keep the static base class (e.g., "dt-chip__avatar"
or the component's base avatar class) and then append avatarClass (e.g.,
:class="['dt-chip__avatar', avatarClass]"); update both occurrences that bind
avatarClass (the one near the top using avatarClass and the second occurrence
around the 166-172 block) so default chip avatar styling is preserved while
still allowing custom classes.
Contributor
Author
There was a problem hiding this comment.
Not a bug. The original avatar span had no base class (), and no d-chip__avatar CSS class exists. :class="avatarClass" is correct.
Contributor
There was a problem hiding this comment.
@iropolo, you're right — the original <span data-qa="dt-chip-avatar"> had no base class at all, so :class="avatarClass" is the correct binding. My suggestion was mistaken. Apologies for the noise!
(ᵔᴥᵔ)
✏️ Learnings added
Learnt from: iropolo
Repo: dialpad/dialtone PR: 1214
File: packages/dialtone-vue/components/chip/chip.vue:25-25
Timestamp: 2026-04-20T18:47:06.948Z
Learning: In `packages/dialtone-vue/components/chip/chip.vue`, the avatar wrapper span (`data-qa="dt-chip-avatar"`) intentionally has no base CSS class — there is no `d-chip__avatar` class. The `:class="avatarClass"` binding is correct as-is; do not suggest adding a base class to this element.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: francisrupert
Repo: dialpad/dialtone PR: 1199
File: packages/combinator/src/lib/tokens.js:63-64
Timestamp: 2026-04-15T19:43:14.186Z
Learning: In dialpad/dialtone's Combinator (`packages/combinator/src/lib/tokens.js`), the avatar `family` color swatch intentionally resolves against a hardcoded `data-avatar-variant="3"` to represent the mid-range hue for each of the 12 color families. The `variant` prop controls lightness/chroma within a family, not the hue itself, so it is deliberately excluded from the swatch resolver and cache key. Threading `variant` through would add complexity without meaningful benefit to the swatch's purpose.
Learnt from: braddialpad
Repo: dialpad/dialtone PR: 1177
File: packages/dialtone-vue/components/checkbox/checkbox_constants.js:1-5
Timestamp: 2026-04-09T22:23:30.222Z
Learning: In the dialtone repo (packages/dialtone-vue/components/checkbox/checkbox_constants.js), the removal of `error` and `success` keys from `CHECKBOX_INPUT_VALIDATION_CLASSES` (replaced by `critical` and `positive`) is an intentional breaking change. No deprecated alias mappings (error→critical, success→positive) are needed or desired.
Learnt from: francisrupert
Repo: dialpad/dialtone PR: 1194
File: packages/dialtone-css/lib/build/less/components/notice.less:55-59
Timestamp: 2026-04-12T21:13:39.883Z
Learning: Repo: dialpad/dialtone — PR `#1194`. For CSS-internal refactors (e.g., notice/banner/toast moving from descendant selectors to CSS custom property inheritance) that produce identical compiled CSS, maintainers consider this non-behavioral and do not require new tests; a brief compiled-CSS diff or verification note in the PR is acceptable proof.
Learnt from: braddialpad
Repo: dialpad/dialtone PR: 1177
File: packages/dialtone-vue/components/checkbox/checkbox_constants.js:1-5
Timestamp: 2026-04-09T22:23:30.222Z
Learning: In the dialtone repo (PR `#1177`), the broad rename from `error`/`success`/`danger` to `critical`/`positive` across component prop values, CSS class names, and constants is an intentional breaking change with no legacy fallback aliases desired. This applies across DtButton, DtBadge, DtBanner, DtNotice, DtToast, DtModal, DtLink, DtCheckbox, DtRadio, DtInput, DtSelectMenu, and related validation message constants.
Learnt from: braddialpad
Repo: dialpad/dialtone PR: 1177
File: packages/dialtone-vue/common/mixins/input.test.js:24-24
Timestamp: 2026-04-09T22:23:21.554Z
Learning: In the dialtone monorepo’s packages/dialtone-vue, treat the validationState rename as an intentional breaking change: `VALIDATION_MESSAGE_TYPES.ERROR`→`CRITICAL` and `VALIDATION_MESSAGE_TYPES.SUCCESS`→`POSITIVE` should use the new `'critical'`/`'positive'` string values, and do not require backward compatibility with the prior `'error'`/`'success'` values or test coverage for the old strings.
Learnt from: braddialpad
Repo: dialpad/dialtone PR: 1177
File: packages/dialtone-vue/components/banner/banner.vue:121-126
Timestamp: 2026-04-09T22:23:11.268Z
Learning: In the dialtone repo (packages/dialtone-vue/components/banner/banner.vue), the removal of `error` and `success` as valid `kind` prop values for DtBanner is an intentional breaking change. No legacy fallback mapping (error→critical, success→positive) is needed or desired for this component.
Learnt from: braddialpad
Repo: dialpad/dialtone PR: 1177
File: packages/dialtone-vue/components/split_button/split_button.test.js:98-105
Timestamp: 2026-04-09T22:26:01.570Z
Learning: In the dialtone repo (packages/dialtone-vue/components/split_button/split_button.test.js and split_button_constants.js), the removal of `danger` as a valid `kind` value for DtSplitButton (replaced by `critical`) is an intentional breaking change. No deprecated alias mapping (danger→critical) is needed or desired, and no backward-compatibility test coverage for the old `danger` string is required.
# Conflicts: # packages/dialtone-vue/components/banner/banner.vue # packages/dialtone-vue/components/notice/notice.vue # packages/dialtone-vue/components/toast/layouts/toast_layout_alternate.vue # packages/dialtone-vue/components/toast/layouts/toast_layout_default.vue
Contributor
|
Please add either the |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/dialtone-vue/components/notice/notice.vue (1)
162-183: Consider tightening prop comments for accepted class shapes.
The comments say “class name,” but these props acceptString | Array | Object; “Additional classes…” would be more precise.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/dialtone-vue/components/notice/notice.vue` around lines 162 - 183, Update the prop comments for iconClass, headerClass, and actionClass to reflect that they accept multiple class shapes (String | Array | Object) rather than a single class name; change phrasing from "Additional class name for the ... element." to something like "Additional classes for the ... element (String | Array | Object)." This clarifies accepted types for the iconClass, headerClass, and actionClass props.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/dialtone-vue/components/notice/notice.vue`:
- Around line 162-183: Update the prop comments for iconClass, headerClass, and
actionClass to reflect that they accept multiple class shapes (String | Array |
Object) rather than a single class name; change phrasing from "Additional class
name for the ... element." to something like "Additional classes for the ...
element (String | Array | Object)." This clarifies accepted types for the
iconClass, headerClass, and actionClass props.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 86c4d0ac-c242-4c85-a9c7-289a732a7813
📒 Files selected for processing (7)
packages/dialtone-vue/components/banner/banner.vuepackages/dialtone-vue/components/modal/modal.vuepackages/dialtone-vue/components/notice/notice.vuepackages/dialtone-vue/components/notice/notice_content.vuepackages/dialtone-vue/components/toast/layouts/toast_layout_alternate.vuepackages/dialtone-vue/components/toast/layouts/toast_layout_default.vuepackages/dialtone-vue/components/toast/toast.vue
✅ Files skipped from review due to trivial changes (1)
- packages/dialtone-vue/components/modal/modal.vue
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/dialtone-vue/components/notice/notice_content.vue
- packages/dialtone-vue/components/banner/banner.vue
- packages/dialtone-vue/components/toast/layouts/toast_layout_alternate.vue
- packages/dialtone-vue/components/toast/layouts/toast_layout_default.vue
Ignacio Ropolo (iropolo)
added
the
no-visual-test
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/dialtone-vue/components/empty_state/empty_state.test.js (1)
239-249: Keep one assertion in this test caseLine [247] and Line [248] currently assert two outcomes in one test. Please split or collapse to a single behavioral assertion per test.
♻️ Suggested update
it('Should apply bodyClass to the body slot wrapper', async () => { mockProps = { bodyClass: 'my-body-class' }; mockSlots = { body: MOCK_BODY_SLOT }; await updateWrapper(); - const bodyWrapper = wrapper.find('.my-body-class'); - - expect(bodyWrapper.exists()).toBe(true); - expect(bodyWrapper.find('h2').exists()).toBe(true); + expect(wrapper.find('.my-body-class h2').exists()).toBe(true); });As per coding guidelines, "
packages/dialtone-vue/**/*.test.js: - Tests use Vitest +@vue/test-utils. - One assertion per test. Use beforeAll/beforeEach to reduce repetition. Use it.each for similar tests with different values."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/dialtone-vue/components/empty_state/empty_state.test.js` around lines 239 - 249, The test "Should apply bodyClass to the body slot wrapper" currently asserts two things; change it to assert only that the element with the passed class exists (keep mockProps = { bodyClass: 'my-body-class' }, mockSlots = { body: MOCK_BODY_SLOT }, call updateWrapper(), then expect(wrapper.find('.my-body-class').exists()).toBe(true)). Move the second assertion (checking the slot content: bodyWrapper.find('h2').exists()) into a separate test (e.g., "renders body slot content inside body wrapper") that reuses the same setup (or use beforeEach/updateWrapper) and only asserts bodyWrapper.find('h2').exists(). This keeps each test to one assertion and references updateWrapper, wrapper.find('.my-body-class'), and bodyWrapper.find('h2').exists().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/dialtone-vue/components/empty_state/empty_state.test.js`:
- Around line 239-249: The test "Should apply bodyClass to the body slot
wrapper" currently asserts two things; change it to assert only that the element
with the passed class exists (keep mockProps = { bodyClass: 'my-body-class' },
mockSlots = { body: MOCK_BODY_SLOT }, call updateWrapper(), then
expect(wrapper.find('.my-body-class').exists()).toBe(true)). Move the second
assertion (checking the slot content: bodyWrapper.find('h2').exists()) into a
separate test (e.g., "renders body slot content inside body wrapper") that
reuses the same setup (or use beforeEach/updateWrapper) and only asserts
bodyWrapper.find('h2').exists(). This keeps each test to one assertion and
references updateWrapper, wrapper.find('.my-body-class'), and
bodyWrapper.find('h2').exists().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 3b63b3c5-8902-443b-8c60-b5da9a13a9a9
📒 Files selected for processing (5)
packages/dialtone-vue/components/empty_state/empty_state.stories.jspackages/dialtone-vue/components/empty_state/empty_state.test.jspackages/dialtone-vue/components/popover/popover.stories.jspackages/dialtone-vue/components/popover/popover.test.jspackages/dialtone-vue/components/toast/layouts/toast_layout_alternate.vue
✅ Files skipped from review due to trivial changes (3)
- packages/dialtone-vue/components/popover/popover.stories.js
- packages/dialtone-vue/components/empty_state/empty_state.stories.js
- packages/dialtone-vue/components/popover/popover.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/dialtone-vue/components/toast/layouts/toast_layout_alternate.vue
Ignacio Ropolo (iropolo)
requested review from
Brad Paugh (braddialpad),
Francis Rupert (francisrupert) and
Nina Repetto (ninarepetto)
as code owners
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 49e9e3d3c5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "Codex (@codex) address that feedback".
| v-if="hasSlotContent($slots.header)" | ||
| :id="labelledById" | ||
| class="d-modal__header" | ||
| :class="['d-modal__header', headerClass]" |
There was a problem hiding this comment.
The new headerClass prop is only applied when the header slot exists, but not when the modal renders its fallback headerText branch. In the common case where consumers pass headerText without a header slot, headerClass is silently ignored, so the feature added in this change does not work for that usage path.
Useful? React with 👍 / 👎.
Brad Paugh (braddialpad)
requested changes
Apr 20, 2026
Contributor
|
Some more suggestions from AI on some missed slots. Some of them may be intended so implement at your discretion: 1.
|
added 2 commits
April 20, 2026 19:18
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/dialtone-vue/components/box/box.vue`:
- Around line 224-229: Add the new scrollbarContentClass prop across the six
artifacts: in box.stories.js add a scrollbarContentClass entry to argTypesData
with a control type (string) and a concise description matching the component
prop; in box.test.js add a test in the DtBox suite that mounts DtBox with
scrollbar and scrollbarContentClass set and asserts the provided class is
present on the scrollbar content wrapper element (target the wrapper by its
class or role used in the component); and update
apps/dialtone-documentation/docs/components/box.md to document the
scrollbarContentClass prop (type, default, and brief description) so docs,
stories, and tests reflect the new prop.
In `@packages/dialtone-vue/components/breadcrumbs/breadcrumbs.vue`:
- Around line 68-74: Add the new listClass prop to Storybook argTypes and tests:
update breadcrumbs.stories.js to include a listClass argType (matching types
used for other class props) and add examples in the existing .story.vue
templates showing its effect; then add a unit test in breadcrumb.test.js that
renders the Breadcrumbs component with listClass set to a test CSS class and
asserts the list wrapper element receives that class (refer to the prop name
listClass and the test helper/render function used in breadcrumb.test.js and the
story templates to mirror usage).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 1131bcd8-e7ba-4c14-bd52-38efe37c4115
📒 Files selected for processing (4)
packages/dialtone-vue/components/box/box.vuepackages/dialtone-vue/components/breadcrumbs/breadcrumbs.vuepackages/dialtone-vue/components/filter_pill/filter_pill.vuepackages/dialtone-vue/components/resizable/resizable_panel.vue
Comment on lines
+224
to
+229
|
|
||
| /** | ||
| * Additional CSS classes applied to the scrollbar content wrapper element. | ||
| * Only applies when scrollbar prop is set. | ||
| */ | ||
| scrollbarContentClass: { type: [String, Object, Array], default: '' }, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Find all references to the new prop
rg -n -C2 '\bscrollbarContentClass\b'
# 2) Locate likely DtBox artifacts expected by repo conventions
fd -i 'box.vue'
fd -i 'box.test.js'
fd -i 'box.stories.js'
fd -i 'box.mdx'
fd -i '*box*.json'
fd -i '*box*.md'
# 3) Check whether those artifacts mention the new prop
rg -n -C2 '\bscrollbarContentClass\b' --iglob '*box*'Repository: dialpad/dialtone
Length of output: 1891
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== 1. Check box.test.js for scrollbarContentClass ==="
rg -n 'scrollbarContentClass' packages/dialtone-vue/components/box/box.test.js || echo "Not found in test file"
echo -e "\n=== 2. Check box.stories.js for scrollbarContentClass ==="
rg -n 'scrollbarContentClass' packages/dialtone-vue/components/box/box.stories.js || echo "Not found in stories file"
echo -e "\n=== 3. Find box component docs JSON files ==="
find packages/dialtone-vue/components/box -name "*.json" -type f 2>/dev/null || echo "No JSON files found"
echo -e "\n=== 4. Find box component VuePress docs ==="
find . -path "*/docs*" -name "*box*" -type f 2>/dev/null | head -20
echo -e "\n=== 5. Search for MCP server data files ==="
find . -name "*mcp*" -o -name "*meta*" -o -name "*schema*" 2>/dev/null | grep -i box | head -10
echo -e "\n=== 6. Check if there's a component docs registry ==="
rg -l 'box.*component\|component.*box' --type json --type js 2>/dev/null | grep -i 'doc\|meta\|schema' | head -10Repository: dialpad/dialtone
Length of output: 1573
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check VuePress box.md for scrollbarContentClass ==="
rg -n 'scrollbarContentClass' ./apps/dialtone-documentation/docs/components/box.md || echo "Not found in VuePress docs"
echo -e "\n=== Check docs/_data/ for component metadata ==="
ls -la ./apps/dialtone-documentation/docs/_data/ 2>/dev/null || echo "Directory not found"
echo -e "\n=== Search for component JSON metadata files ==="
find ./apps/dialtone-documentation -name "*box*.json" -type f 2>/dev/null
echo -e "\n=== Search for MCP server or metadata patterns ==="
find . -type f -name "*.json" -exec rg -l '"box".*"props"|"scrollbarContentClass"' {} \; 2>/dev/null | head -5
echo -e "\n=== Check if docs/_data/checkbox.json shows the expected structure ==="
head -50 ./apps/dialtone-documentation/docs/_data/checkbox.json 2>/dev/null || echo "File not found"Repository: dialpad/dialtone
Length of output: 5347
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check box.test.js to see if it should document props ==="
head -80 packages/dialtone-vue/components/box/box.test.js
echo -e "\n=== Check box.stories.js to see if it should document props ==="
head -80 packages/dialtone-vue/components/box/box.stories.js
echo -e "\n=== Check if there are .mdx files for box component ==="
find packages/dialtone-vue/components/box -name "*.mdx" -type f
echo -e "\n=== Check for TypeScript definitions or prop documentation patterns ==="
find packages/dialtone-vue/components/box -type f | head -20Repository: dialpad/dialtone
Length of output: 4570
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for complete argTypesData in box.stories.js ==="
rg -A 200 'argTypesData' packages/dialtone-vue/components/box/box.stories.js | grep -E '(scrollbar|}})'
echo -e "\n=== Check if scrollbar tests exist in box.test.js ==="
rg -n 'scrollbar' packages/dialtone-vue/components/box/box.test.js
echo -e "\n=== Check box.vue for full prop list with scrollbar ==="
rg -B 5 -A 5 'scrollbar' packages/dialtone-vue/components/box/box.vue | head -60
echo -e "\n=== Check if box.stories.js mentions scrollbarContent anywhere ==="
rg -n 'scrollbar' packages/dialtone-vue/components/box/box.stories.jsRepository: dialpad/dialtone
Length of output: 118
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Read full box.stories.js file ==="
wc -l packages/dialtone-vue/components/box/box.stories.js
echo -e "\n=== Get argTypesData section from box.stories.js ==="
sed -n '/export const argTypesData/,/^}/p' packages/dialtone-vue/components/box/box.stories.js | tail -80
echo -e "\n=== Search for scrollbar in box.test.js ==="
rg -n 'scrollbar' packages/dialtone-vue/components/box/box.test.js || echo "No scrollbar tests found"
echo -e "\n=== List all props in box.vue ==="
sed -n '/defineProps/,/^[});]/p' packages/dialtone-vue/components/box/box.vue | head -100Repository: dialpad/dialtone
Length of output: 7170
Update all DtBox artifacts for the new scrollbarContentClass prop.
The prop was added to the Vue component but is missing from Storybook controls (argTypesData in box.stories.js), tests (box.test.js), and VuePress documentation (box.md). Per the 6-artifact sync requirement, add:
scrollbarContentClassentry toargTypesDatain box.stories.js (with control type and description)- Test case in box.test.js verifying the class is applied to the scrollbar content wrapper
- Documentation in apps/dialtone-documentation/docs/components/box.md
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/dialtone-vue/components/box/box.vue` around lines 224 - 229, Add the
new scrollbarContentClass prop across the six artifacts: in box.stories.js add a
scrollbarContentClass entry to argTypesData with a control type (string) and a
concise description matching the component prop; in box.test.js add a test in
the DtBox suite that mounts DtBox with scrollbar and scrollbarContentClass set
and asserts the provided class is present on the scrollbar content wrapper
element (target the wrapper by its class or role used in the component); and
update apps/dialtone-documentation/docs/components/box.md to document the
scrollbarContentClass prop (type, default, and brief description) so docs,
stories, and tests reflect the new prop.
Comment on lines
+68
to
+74
| /** | ||
| * Additional CSS class(es) applied to the list wrapper element. | ||
| */ | ||
| listClass: { | ||
| type: [String, Array, Object], | ||
| default: '', | ||
| }, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify references for newly introduced class props in this PR scope.
# Expected: references in component tests, stories, docs JSON/VuePress/MCP data where applicable.
echo "== Candidate component-related files =="
fd -i 'breadcrumbs|filter_pill' packages
echo "== Prop references in likely companion artifacts =="
rg -n -C2 --iglob '*breadcrumbs*' --iglob '*filter_pill*' '\b(listClass|labelClass)\b' packagesRepository: dialpad/dialtone
Length of output: 3528
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check for breadcrumbs test file =="
fd 'breadcrumbs' packages/dialtone-vue --type f | grep -i test
echo "== Search for listClass in breadcrumbs test (if exists) =="
find packages/dialtone-vue/components/breadcrumbs -name '*.test.js' -o -name '*.spec.js' | while read f; do
if [ -f "$f" ]; then
echo "Found: $f"
rg -n 'listClass' "$f" || echo "No listClass references found"
fi
done
echo "== Search for listClass in breadcrumbs story files =="
rg -n 'listClass' packages/dialtone-vue/components/breadcrumbs/ || echo "No listClass in breadcrumbs stories"
echo "== Check for component docs metadata files =="
find packages -name '*breadcrumbs*' -type f | grep -E '\.(json|mdx?)$' | head -20
echo "== Check for VuePress docs =="
find packages -path '*/docs/*' -name '*breadcrumb*' 2>/dev/null || echo "No VuePress breadcrumb docs found"
echo "== Check MCP server data =="
rg -l 'breadcrumbs' packages --type json | head -10Repository: dialpad/dialtone
Length of output: 97
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== List all files in breadcrumbs directory =="
ls -la packages/dialtone-vue/components/breadcrumbs/
echo "== Check breadcrumbs.stories.js for listClass =="
rg -n 'listClass' packages/dialtone-vue/components/breadcrumbs/breadcrumbs.stories.js || echo "Not found in stories.js"
echo "== Check breadcrumbs story templates for listClass =="
rg -n 'listClass' packages/dialtone-vue/components/breadcrumbs/*.story.vue || echo "Not found in story.vue files"
echo "== Search for component docs/metadata files =="
find packages -name '*.json' -type f | xargs rg -l 'breadcrumbs' 2>/dev/null | head -5
echo "== Check for MDX documentation =="
find packages -name '*.mdx' -type f | xargs rg -l 'breadcrumbs' 2>/dev/null | head -5Repository: dialpad/dialtone
Length of output: 1380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check breadcrumb.test.js for listClass =="
rg -n 'listClass' packages/dialtone-vue/components/breadcrumbs/breadcrumb.test.js || echo "Not found in breadcrumb.test.js"
echo "== Read breadcrumb.test.js to see what it tests =="
head -50 packages/dialtone-vue/components/breadcrumbs/breadcrumb.test.js
echo "== Search for component docs registry JSON =="
find packages/dialtone-vue -name '*docs*' -o -name '*registry*' -o -name '*components*' | grep -i json | head -10
echo "== Check dialtone-vue package for component metadata =="
ls -la packages/dialtone-vue/*.json 2>/dev/null || echo "No JSON files at package root"
echo "== Search for MCP data or VuePress in docs =="
find packages -type d -name 'docs' -o -type d -name '.mcp' | head -5Repository: dialpad/dialtone
Length of output: 1204
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for all JSON files that might contain component metadata =="
find packages -type f -name '*.json' | xargs grep -l '"breadcrumbs"' 2>/dev/null | head -10
echo "== Check if there's a component registry or docs folder =="
ls -la packages/dialtone-vue/docs 2>/dev/null || echo "No docs folder at package root"
echo "== Look for .mdx files related to breadcrumbs =="
find packages -name '*.mdx' | xargs grep -l 'breadcrumb' 2>/dev/null || echo "No MDX breadcrumb docs"
echo "== Check breadcrumbs.stories.js content for completeness =="
cat -n packages/dialtone-vue/components/breadcrumbs/breadcrumbs.stories.js
echo "== Check breadcrumbs_variants.story.vue content =="
cat -n packages/dialtone-vue/components/breadcrumbs/breadcrumbs_variants.story.vueRepository: dialpad/dialtone
Length of output: 131
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Direct read of breadcrumbs.stories.js =="
cat packages/dialtone-vue/components/breadcrumbs/breadcrumbs.stories.js
echo ""
echo "== Direct read of breadcrumbs_variants.story.vue =="
cat packages/dialtone-vue/components/breadcrumbs/breadcrumbs_variants.story.vue
echo ""
echo "== Check entire breadcrumb.test.js for prop coverage =="
cat packages/dialtone-vue/components/breadcrumbs/breadcrumb.test.js | head -100Repository: dialpad/dialtone
Length of output: 5080
Add listClass to Storybook argTypes and test coverage.
The new listClass prop is defined in the component but missing from:
breadcrumbs.stories.jsargTypesbreadcrumb.test.jstest cases- Story template examples in
.story.vuefiles
Update argTypes to include listClass alongside other props, add a test case verifying the prop applies the CSS class, and showcase it in story variants for consistency with other class props.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/dialtone-vue/components/breadcrumbs/breadcrumbs.vue` around lines 68
- 74, Add the new listClass prop to Storybook argTypes and tests: update
breadcrumbs.stories.js to include a listClass argType (matching types used for
other class props) and add examples in the existing .story.vue templates showing
its effect; then add a unit test in breadcrumb.test.js that renders the
Breadcrumbs component with listClass set to a test CSS class and asserts the
list wrapper element receives that class (refer to the prop name listClass and
the test helper/render function used in breadcrumb.test.js and the story
templates to mirror usage).
Contributor
|
✔️ Deploy previews ready! |
Brad Paugh (braddialpad)
approved these changes
Apr 21, 2026
Contributor
Brad Paugh (braddialpad)
left a comment
Brad Paugh (braddialpad)
left a comment
There was a problem hiding this comment.
thanks!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add slot classes
🛠️ Type Of Change
These types will increment the version number on release:
📖 Jira Ticket
https://dialpad.atlassian.net/browse/DLT-3096
📖 Description
Whenever we have a slot, we should add a class named {slowName}Class (e.g. labelClass) .
Renamed Prop (with deprecated fallback)
New Slot Class Props (direct)
New Slot Class Props (pass-through chains)