refactor(components): DLT-3100 remove rootClass references#1184
refactor(components): DLT-3100 remove rootClass references#1184Ignacio Ropolo (iropolo) merged 7 commits intonextfrom
Conversation
|
Please add either the |
Ignacio Ropolo (iropolo)
added
the
no-visual-test
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: ca84a76137
ℹ️ 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".
| :class="['d-input__root', { 'd-input--hidden': hidden }, $attrs.class]" | ||
| :style="$attrs.style" |
There was a problem hiding this comment.
This change makes root styling depend on $attrs.class and removes rootClass, but there are still in-repo callers using root-class (for example packages/dialtone-vue/recipes/conversation_view/editor/editor.vue and packages/combinator/src/components/option_bar/*.vue). Because this component has inheritAttrs: false, those legacy attrs are no longer interpreted as root classes, so expected wrapper styling is dropped and layouts regress in those screens. Please either keep a temporary rootClass compatibility path or update those call sites in the same PR.
Useful? React with 👍 / 👎.
| compatConfig: { MODE: 3 }, | ||
| name: 'DtCard', | ||
|
|
||
| inheritAttrs: false, |
There was a problem hiding this comment.
Adding inheritAttrs: false here drops all non-prop attributes unless they are manually rebound, and the template currently only reapplies class/style. That means consumer attributes like id, data-*, and aria-* no longer reach the root card element, which can break selectors and accessibility hooks. Forwarding the remaining $attrs to the root (while handling class/style explicitly) avoids this regression.
Useful? React with 👍 / 👎.
Ignacio Ropolo (@iropolo) I took the liberty of creating DLT-3281 as a followup action item within DLT-2980, including a speculative planning document. This is a highly repeatable pattern that should be fairly straight-forward to provide a codemod-like migration script, and even lint with autofix. When I've created the migration scripts on others within 
|
Then should DLT-3100 be broken out into multiple stories? Make sense to have one that is solely for this |
|
Not sure |
|
I'm sure you're treating it this way anyway, but would it be prudent to connect https://dialpad.atlassian.net/browse/DLT-3096 as blocked by this one, https://dialpad.atlassian.net/browse/DLT-3100? |
|
Be sure to test them all out visually. Just my first test on a few shows it applying to both the root and the internal part, e.g. Input Checkbox Radio I won't show them all, but you get the idea. |
Claude's reaction:
|
I can change it for refactor, it was the old title before I split the task.
Sure, will be the way to go.
Yeah, no big deal Im on both.
Im on it, thanks for the images! |
Ignacio Ropolo (iropolo)
changed the title
Francis Rupert (francisrupert)
left a comment
Francis Rupert (francisrupert)
left a comment
There was a problem hiding this comment.
Approved from my design perspective. I tested a bunch and can't seem to break it.
Following through on https://dialpad.atlassian.net/browse/DLT-3281 will make this a breeze.
Thanks!
|
CodeRabbit (@coderabbitai) review |
✅ Actions performedReview triggered.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughReplaces component-specific class props (rootClass, wrapperClass, containerClass) and the addClassStyleAttrs helper with standard Vue attribute forwarding: components now use $attrs.class and $attrs.style; stories, docs, tests, variants, and utilities updated accordingly. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/dialtone-vue/common/utils/index.js (1)
247-251:⚠️ Potential issue | 🟠 MajorMigrate 7 recipe components or make
addClassStyleAttrsunconditionalWhen
configVue2StyleClassAttrsis false,addClassStyleAttrsreturns empty object, causing 7 components to silently drop inherited class/style attributes:
callbar_button.vue,callbar_button_with_dropdown.vue,callbar_button_with_popover.vue(missing root:stylebinding)editor.vue,message_input.vue,contact_centers_row.vue,general_row.vue(need verification of full class/style coverage)Recommend either (a) explicitly add
:class="$attrs.class"and:style="$attrs.style"to the root element of each recipe, or (b) makeaddClassStyleAttrsunconditional by removing the config guard, as suggested in the original comment. Additionally, add test coverage for theconfigVue2StyleClassAttrsflag transition behavior to prevent regression.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/dialtone-vue/common/utils/index.js` around lines 247 - 251, The recipe components are losing root class/style when configVue2StyleClassAttrs is false because addClassStyleAttrs can return an empty object; fix by either (A) making addClassStyleAttrs unconditional (remove the config guard so it always returns class/style entries) or (B) explicitly bind the attrs on each affected component root (add :class="$attrs.class" and :style="$attrs.style" to callbar_button.vue, callbar_button_with_dropdown.vue, callbar_button_with_popover.vue, editor.vue, message_input.vue, contact_centers_row.vue, and general_row.vue). Also add unit tests covering the configVue2StyleClassAttrs true→false transition to assert inherited class/style are preserved. Ensure changes reference removeClassStyleAttrs/addClassStyleAttrs functions when updating logic or tests.packages/dialtone-vue/components/split_button/split_button.test.js (1)
660-667:⚠️ Potential issue | 🟡 MinorAdd regression coverage for root style merging.
This test covers class passthrough, but the companion behavior (:style="[$attrs.style, { width }]"insplit_button.vue) is non-trivial and currently unverified here.Suggested test addition
describe('Extendability Tests', () => { describe('When a class is provided', () => { it('should include the class', () => { mockAttrs = { class: MOCK_ROOT_CLASS } updateWrapper(); expect(wrapper.classes().includes(MOCK_ROOT_CLASS)).toBe(true); }); }); + + describe('When style attrs and width are provided', () => { + it('should merge incoming style with width style', () => { + mockProps = { width: '240px' }; + mockAttrs = { style: { color: 'red' } }; + + updateWrapper(); + + const style = wrapper.attributes('style'); + expect(style).toContain('color: red'); + expect(style).toContain('width: 240px'); + }); + });As per coding guidelines: "Flag missing tests for non-trivial logic changes — untested behavior is unverified behavior."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/dialtone-vue/components/split_button/split_button.test.js` around lines 660 - 667, Add a test in split_button.test.js alongside the class passthrough test to verify root style merging: set mockAttrs = { style: { color: 'red' } } (or inline style string), set the component prop width (e.g., via updateWrapper props) and call updateWrapper(), then assert the rendered wrapper's style includes both the provided style and the width (matching split_button.vue's :style="[$attrs.style, { width }]" behavior) using wrapper.attributes('style') or equivalent; reference mockAttrs, updateWrapper, wrapper and the split_button.vue :style merging to locate where to add the test.
🤖 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/card/card.vue`:
- Around line 3-4: The Card component currently sets inheritAttrs: false and
only forwards class and style via :class="['d-card', $attrs.class]" and
:style="$attrs.style", which drops other root attrs and listeners; change the
template to merge all remaining attrs onto the root by spreading $attrs (e.g.,
replace manual :class/:style usage with :class="['d-card', $attrs.class]" and
v-bind="$attrs" on the root or use v-bind="{...$attrs}" while still preserving
the d-card class), ensure inheritAttrs: false remains if needed, and update any
event bindings to use v-on="$listeners" or v-on="listeners equivalent" if using
Vue 2/3 patterns; add a regression test in card.test.js that mounts the Card and
asserts a non-class attribute (e.g., aria-label or data-test) and an attached
event listener are present on the root element.
In `@packages/dialtone-vue/components/input/input.vue`:
- Around line 4-5: The change routes $attrs.class and $attrs.style to the root
element in DtInput (see input.vue’s :class "['d-input__root', {
'd-input--hidden': hidden }, $attrs.class]" and :style "$attrs.style"), but
there are no regression tests to assert class/style land only on the root and
are not forwarded to the inner <input>/<textarea>. Add tests that mount DtInput
with custom class and style passed via attrs, then assert the rendered root
element contains the supplied class and style and the inner <input> (and the
<textarea> variant) does NOT have those class/style attributes; include both
input and textarea modes and any hidden-state variation to cover the
'd-input__root'/'d-input--hidden' behavior.
In `@packages/dialtone-vue/components/split_button/split_button.vue`:
- Around line 4-5: The root split_button.vue already applies incoming
class/style to the root but the same $attrs are still being forwarded to the
inner split-button-start and split-button-end via the computed props (e.g., the
computed props used for start and end children); update those computed props so
they explicitly omit class and style (and any width) from forwarded
attributes—e.g., destructure $attrs to remove class and style and then return
the rest, or build startProps/endProps by picking only allowed keys—so consumer
class/style are applied only to the root and not duplicated on
split-button-start or split-button-end.
In
`@packages/dialtone-vue/recipes/conversation_view/feed_item_pill/feed_item_pill.vue`:
- Around line 2-4: Add a focused unit test in
packages/dialtone-vue/recipes/conversation_view/feed_item_pill/feed_item_pill.test.js
that mounts the FeedItemPill component (import default from feed_item_pill.vue)
with custom attrs class and style passed via attrs or via mounting wrapper, and
assert that the rendered root element (the div with class
'd-recipe-feed-item-pill__border') contains the provided class string and inline
style string; use Vue Test Utils (mount/shallowMount) to locate the root div and
assert its classList includes the forwarded class and its style.cssText (or
style property) matches the forwarded style. Ensure the test covers both class
forwarding and style forwarding in one spec.
---
Outside diff comments:
In `@packages/dialtone-vue/common/utils/index.js`:
- Around line 247-251: The recipe components are losing root class/style when
configVue2StyleClassAttrs is false because addClassStyleAttrs can return an
empty object; fix by either (A) making addClassStyleAttrs unconditional (remove
the config guard so it always returns class/style entries) or (B) explicitly
bind the attrs on each affected component root (add :class="$attrs.class" and
:style="$attrs.style" to callbar_button.vue, callbar_button_with_dropdown.vue,
callbar_button_with_popover.vue, editor.vue, message_input.vue,
contact_centers_row.vue, and general_row.vue). Also add unit tests covering the
configVue2StyleClassAttrs true→false transition to assert inherited class/style
are preserved. Ensure changes reference removeClassStyleAttrs/addClassStyleAttrs
functions when updating logic or tests.
In `@packages/dialtone-vue/components/split_button/split_button.test.js`:
- Around line 660-667: Add a test in split_button.test.js alongside the class
passthrough test to verify root style merging: set mockAttrs = { style: { color:
'red' } } (or inline style string), set the component prop width (e.g., via
updateWrapper props) and call updateWrapper(), then assert the rendered
wrapper's style includes both the provided style and the width (matching
split_button.vue's :style="[$attrs.style, { width }]" behavior) using
wrapper.attributes('style') or equivalent; reference mockAttrs, updateWrapper,
wrapper and the split_button.vue :style merging to locate where to add the test.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: a9f9d60d-f58f-443e-8034-9040fa2ff9e4
📒 Files selected for processing (40)
apps/dialtone-documentation/docs/.vuepress/baseComponents/ClampedTableWrapper.vueapps/dialtone-documentation/docs/components/button.mdapps/dialtone-documentation/docs/components/toggle.mdpackages/combinator/.github/documentation/USAGE.mdpackages/combinator/src/components/combinator.vuepackages/combinator/src/components/controls/control_boolean.vuepackages/combinator/src/components/option_bar/option_bar.vuepackages/combinator/src/components/option_bar/option_bar_control.vuepackages/combinator/src/variants/variants_card.jspackages/combinator/src/variants/variants_toggle.jspackages/dialtone-vue/common/mixins/input.jspackages/dialtone-vue/common/utils/index.jspackages/dialtone-vue/components/breadcrumbs/breadcrumb.test.jspackages/dialtone-vue/components/breadcrumbs/breadcrumb_item.vuepackages/dialtone-vue/components/breadcrumbs/breadcrumb_item_default.story.vuepackages/dialtone-vue/components/card/card.vuepackages/dialtone-vue/components/card/card_default.story.vuepackages/dialtone-vue/components/checkbox/checkbox.test.jspackages/dialtone-vue/components/checkbox/checkbox.vuepackages/dialtone-vue/components/checkbox/checkbox_default.story.vuepackages/dialtone-vue/components/input/input.test.jspackages/dialtone-vue/components/input/input.vuepackages/dialtone-vue/components/input/input_default.story.vuepackages/dialtone-vue/components/radio/radio.test.jspackages/dialtone-vue/components/radio/radio.vuepackages/dialtone-vue/components/radio/radio_default.story.vuepackages/dialtone-vue/components/select_menu/select_menu.test.jspackages/dialtone-vue/components/select_menu/select_menu.vuepackages/dialtone-vue/components/select_menu/select_menu_default.story.vuepackages/dialtone-vue/components/split_button/split_button.test.jspackages/dialtone-vue/components/split_button/split_button.vuepackages/dialtone-vue/components/split_button/split_button_default.story.vuepackages/dialtone-vue/components/toggle/toggle.stories.jspackages/dialtone-vue/components/toggle/toggle.vuepackages/dialtone-vue/components/toggle/toggle_default.story.vuepackages/dialtone-vue/recipes/conversation_view/feed_item_pill/feed_item_pill.stories.jspackages/dialtone-vue/recipes/conversation_view/feed_item_pill/feed_item_pill.vuepackages/dialtone-vue/recipes/conversation_view/feed_item_pill/feed_item_pill_default.story.vuepackages/dialtone-vue/recipes/conversation_view/feed_item_pill/feed_item_pill_variants.story.vuepackages/dialtone-vue/recipes/conversation_view/feed_item_row/feed_item_row_variants.story.vue
💤 Files with no reviewable changes (2)
- packages/dialtone-vue/common/mixins/input.js
- packages/combinator/src/components/combinator.vue
| :class="['d-input__root', { 'd-input--hidden': hidden }, $attrs.class]" | ||
| :style="$attrs.style" |
There was a problem hiding this comment.
Add regression tests for root-only class/style forwarding.
Line 4 and Line 5 change core attr-routing behavior on DtInput. I don’t see matching regression coverage in the provided changes to assert class/style land on the root container and are not forwarded to inner <input>/<textarea>. Please add or point to tests before merge.
As per coding guidelines, "Flag missing tests for non-trivial logic changes — untested behavior is unverified behavior."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/dialtone-vue/components/input/input.vue` around lines 4 - 5, The
change routes $attrs.class and $attrs.style to the root element in DtInput (see
input.vue’s :class "['d-input__root', { 'd-input--hidden': hidden },
$attrs.class]" and :style "$attrs.style"), but there are no regression tests to
assert class/style land only on the root and are not forwarded to the inner
<input>/<textarea>. Add tests that mount DtInput with custom class and style
passed via attrs, then assert the rendered root element contains the supplied
class and style and the inner <input> (and the <textarea> variant) does NOT have
those class/style attributes; include both input and textarea modes and any
hidden-state variation to cover the 'd-input__root'/'d-input--hidden' behavior.
| :class="['d-split-btn', { 'd-split-btn--no-divider': !showDivider && importance === 'clear' }, $attrs.class]" | ||
| :style="[$attrs.style, { width }]" |
There was a problem hiding this comment.
Prevent class/style from being applied to both root and inner buttons.
Line 4 and Line 5 now correctly apply incoming class/style to the root, but the same attrs are still forwarded to split-button-start and split-button-end via computed props (Line 604-605 and Line 619-620). That duplicates consumer styling across multiple elements and can break selectors/layout unexpectedly.
Proposed fix
startButtonProps () {
return {
@@
- class: this.$attrs.class,
- style: this.$attrs.style,
};
},
endButtonProps () {
return {
@@
- class: this.$attrs.class,
- style: this.$attrs.style,
};
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/dialtone-vue/components/split_button/split_button.vue` around lines
4 - 5, The root split_button.vue already applies incoming class/style to the
root but the same $attrs are still being forwarded to the inner
split-button-start and split-button-end via the computed props (e.g., the
computed props used for start and end children); update those computed props so
they explicitly omit class and style (and any width) from forwarded
attributes—e.g., destructure $attrs to remove class and style and then return
the rest, or build startProps/endProps by picking only allowed keys—so consumer
class/style are applied only to the root and not duplicated on
split-button-start or split-button-end.
There was a problem hiding this comment.
hmm checking this one
There was a problem hiding this comment.
@iropolo — confirmed, the issue is real. The current file shows $attrs.class and $attrs.style applied to the root <span> and forwarded through both computed props:
// startButtonProps
class: this.$attrs.class, // ← duplicate
style: this.$attrs.style, // ← duplicate
// endButtonProps
class: this.$attrs.class, // ← duplicate
style: this.$attrs.style, // ← duplicateConsumer class/style ends up on three elements simultaneously (root span + start button + end button). The fix is to remove those two lines from both computed props so the attrs only land on the root:
startButtonProps () {
return {
active: this.alphaActive ?? this.startActive,
// ... other props ...
// Remove: class: this.$attrs.class,
// Remove: style: this.$attrs.style,
};
},
endButtonProps () {
return {
// ... other props ...
// Remove: class: this.$attrs.class,
// Remove: style: this.$attrs.style,
};
},🧠 Learnings used
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.
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/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: 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/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/modal/modal.test.js:223-233
Timestamp: 2026-04-09T22:23:42.967Z
Learning: In the dialtone repo (packages/dialtone-vue/components/modal/modal_constants.js and modal.test.js), the removal of `error` and `success` as valid `bannerKind` values for DtModal is an intentional breaking change. No legacy fallback mapping (error→critical, success→positive) is needed or desired, and no backward-compatibility test coverage for the old strings is required.
Learnt from: braddialpad
Repo: dialpad/dialtone PR: 1177
File: packages/dialtone-vue/components/select_menu/select_menu_constants.js:16-20
Timestamp: 2026-04-09T22:25:15.170Z
Learning: In the dialtone repo (packages/dialtone-vue/components/select_menu/select_menu_constants.js), the removal of `error` and `success` keys from `SELECT_STATE_MODIFIERS` (replaced by `critical` and `positive`) is an intentional breaking change. No deprecated alias mappings (error→critical, success→positive) are needed or desired.
Learnt from: braddialpad
Repo: dialpad/dialtone PR: 1177
File: packages/dialtone-vue/components/toast/toast.test.js:117-124
Timestamp: 2026-04-09T22:25:13.076Z
Learning: In the dialtone repo (packages/dialtone-vue/components/toast/toast.test.js and related toast files), the removal of `error` and `success` as valid `kind` values for DtToast (replaced by `critical` and `positive`) is an intentional breaking change. No legacy fallback mapping (error→critical, success→positive) is needed or desired, and no backward-compatibility test coverage for the old strings is required.
Learnt from: braddialpad
Repo: dialpad/dialtone PR: 1177
File: packages/dialtone-vue/common/utils/index.js:114-127
Timestamp: 2026-04-09T22:25:51.363Z
Learning: In the dialtone repo (packages/dialtone-vue/common/utils/index.js, PR `#1177`), dropping legacy `error`/`success` message type recognition in `getValidationState` is an intentional breaking change. No normalization/alias mapping from `error`→`critical` or `success`→`positive` is needed or desired.
Learnt from: CR
Repo: dialpad/dialtone PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-10T14:07:40.863Z
Learning: Applies to packages/dialtone-vue/components/**/*.vue : Use `update:open` for new overlay component events and `update:show` for legacy Modal/Tooltip/Toast events
There was a problem hiding this comment.
yeah, this is intentional behaviour, we will still remove the rootClass prop but keep the forwarding to inner buttons.
| <div | ||
| :class="['d-recipe-feed-item-pill__border', borderClass, $attrs.class]" | ||
| :style="$attrs.style" |
There was a problem hiding this comment.
Add focused unit coverage for root class/style forwarding.
This change modifies runtime attr-forwarding behavior, but the provided diff does not include a test proving attrs.class/attrs.style are applied as intended on the root pill container. Please add a test case in packages/dialtone-vue/recipes/conversation_view/feed_item_pill/feed_item_pill.test.js for this path before merge.
As per coding guidelines, "Flag missing tests for non-trivial logic changes — untested behavior is unverified behavior."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/dialtone-vue/recipes/conversation_view/feed_item_pill/feed_item_pill.vue`
around lines 2 - 4, Add a focused unit test in
packages/dialtone-vue/recipes/conversation_view/feed_item_pill/feed_item_pill.test.js
that mounts the FeedItemPill component (import default from feed_item_pill.vue)
with custom attrs class and style passed via attrs or via mounting wrapper, and
assert that the rendered root element (the div with class
'd-recipe-feed-item-pill__border') contains the provided class string and inline
style string; use Vue Test Utils (mount/shallowMount) to locate the root div and
assert its classList includes the forwarded class and its style.cssText (or
style property) matches the forwarded style. Ensure the test covers both class
forwarding and style forwarding in one spec.
There was a problem hiding this comment.
Because card is not an "input" component, we can just remove inheritAttrs and $attrs.class and $attrs.style everything will just go to the root naturally. The same goes for any other components that simply apply $attrs to the root.
There was a problem hiding this comment.
Sure, this would be a good clean up, made it in :2f5b32d
There was a problem hiding this comment.
Alright so.. this sort of thing gets quite confusing simply because of the configVue2StyleClassAttrs option which needs to be true in ubervoice due to the vue 3 migration, but is false everywhere else. In this case when configVue2StyleClassAttrs is false, class and style attrs will be applied to BOTH the root element and the dt-link sub-component. Probably not what we want.
I think to fix this properly across all components we need to do the following:
- Remove the
v-bind="addClassStyleAttrs($attrs)"and replace with direct$attrs.classand$attrs.stylelike you have done here. - Remove the
if (!configVue2StyleClassAttrs) return attrs;condition from theremoveClassStyleAttrsfunction definition. This means it will ALWAYS remove class and style attrs from $attrs regardless of whether configVue2StyleClassAttrs is true or not.
We are basically following vue 2 attrs style after this change, so we shouldn't even need configVue2StyleClassAttrs option anymore.
There was a problem hiding this comment.
Yeah, for 2 I already removed it here eea5fe1
|
✔️ Deploy previews ready! |
Brad Paugh (braddialpad)
left a comment
Brad Paugh (braddialpad)
left a comment
There was a problem hiding this comment.
Thanks, looks good now!
# [1.2.0-next.4](combinator/v1.2.0-next.3...combinator/v1.2.0-next.4) (2026-04-15) ### Code Refactoring * **Components:** DLT-3100 remove rootClass references ([#1184](#1184)) ([6531783](6531783)) * **Text:** DLT-3302 tone prop uses css variables instead of css utilities ([#1197](#1197)) ([75df16f](75df16f)) ### Documentation * DLT-3250 display component status badges in sidebar and page headers ([#1170](#1170)) ([7d5f5d5](7d5f5d5)) * **Playground:** DLT-3242 segmented control for compact enum props ([#1165](#1165)) ([fa440d8](fa440d8)) ### Features * **Components:** DLT-3157 align critical/positive prop values and CSS class names ([#1177](#1177)) ([edd98b2](edd98b2)) * **Split Button:** DLT-3257 add show-divider prop ([#1173](#1173)) ([5bef411](5bef411))
# [3.220.0-next.1](dialtone-vue/v3.219.0...dialtone-vue/v3.220.0-next.1) (2026-04-15) ### Bug Fixes * **Editor:** NO-JIRA remove extra blank line in editor.mdx ([18c47ef](18c47ef)) * **Split Button, Filter Pill:** DLT-3260 DLT-3261 correct misalignment at size 100 ([#1176](#1176)) ([fe2b777](fe2b777)) * **Tabs:** DLT-3251 improve accessibility ([#1171](#1171)) ([01b101e](01b101e)) ### Code Refactoring * **Combobox Multi Select:** NO-JIRA use logical css properties ([#1104](#1104)) ([a675daf](a675daf)) * **Components,recipes:** DLT-3043 DLT-3046 logical start/end naming with deprecated backward-compatible fallbacks ([#1079](#1079)) ([1d26a4e](1d26a4e)) * **Components:** DLT-3100 remove rootClass references ([#1184](#1184)) ([6531783](6531783)) * **Components:** dlt-3282 renamed show prop to open ([#1190](#1190)) ([5509d0f](5509d0f)) * **Components:** dlt-3283 refactor prop/slots to be consistent ([#1196](#1196)) ([2d22c76](2d22c76)) * **Modal:** DLT-3262 migrate DtModal to native dialog element ([#1179](#1179)) ([589edb5](589edb5)) * **Text:** DLT-3302 tone prop uses css variables instead of css utilities ([#1197](#1197)) ([75df16f](75df16f)) * **Tokens,css,link,text,breadcrumbs,input Group,split Button:** tokens, CSS foundation, deprecations, tooling DLT-3011 DLT-2961 DLT-3068 DLT-3069 DLT-3070 DLT-3071 DLT-3072 ([#1092](#1092)) ([7783090](7783090)) * **Tokens:** DLT-3013 convert color system from HSL to OKLCH ([#1060](#1060)) ([42dc418](42dc418)) ### Documentation * **Dialtone Vue:** DLT-3226 update documentation and consuming code to numeric size scale ([#1158](#1158)) ([149c763](149c763)) * **Playground:** DLT-3119 refine playground UI and integrate across doc site ([#1127](#1127)) ([ba6b599](ba6b599)) ### Features * **Avatar:** DLT-2942 updated avatar component ([#1047](#1047)) ([e50563d](e50563d)) * **Avatar:** DLT-3161 change from clickable to "interactive" ([#1182](#1182)) ([2d00fe0](2d00fe0)) * **Banner, Notice, Toast, Breadcrumbs, Checkbox, Radio, Input, Select Menu, Empty State:** visual refresh, dt-text integration, form api parity DLT-2944 DLT-2945 DLT-2949 DLT-2959 DLT-2960 DLT-2957 ([#1091](#1091)) ([16e908a](16e908a)) * **Button,tabs,split Button:** visual refresh and slot updates DLT-2946 DLT-2947 DLT-2965 ([#1090](#1090)) ([ffdd56f](ffdd56f)) * **Components:** DLT-3157 align critical/positive prop values and CSS class names ([#1177](#1177)) ([edd98b2](edd98b2)) * **Components:** DLT-3159 enforce positive boolean props ([#1198](#1198)) ([ac73bdc](ac73bdc)) * **Dialtone Css, Dialtone Tokens, Dialtone Vue:** DLT-3197 DLT-3198 DLT-3199 DLT-3200 DLT-3201 DLT-3202 DLT-3203 DLT-3204 DLT-3205 DLT-3206 DLT-3207 DLT-3208 DLT-3209 DLT-3210 DLT-3211 token-stop utility classes, migration tooling, and documentation ([#1150](#1150)) ([6237b84](6237b84)) * **Dialtone Vue:** DLT-3225 migrate component size props to numeric ordinal scale ([#1157](#1157)) ([725bd41](725bd41)) * **Filter Pill:** DLT-2941 DtFilterPill improvements ([#1110](#1110)) ([23c5689](23c5689)) * **Focusgroup:** DLT-3285 add v-dt-focusgroup directive for declarative roving tabindex ([#1187](#1187)) ([e1ea074](e1ea074)) * **Focustrap:** DLT-3297 add v-dt-focustrap directive for declarative focus trapping ([#1195](#1195)) ([6ef9a21](6ef9a21)) * **Link,button:** DLT-3012 add underline prop ([#1059](#1059)) ([1cb91ea](1cb91ea)) * **Mode Directive, Mode Island:** DLT-3067 DLT-3078 DLT-3077 add `v-dt-mode` directive ([#1109](#1109)) ([f97efca](f97efca)) * **Segmented Control:** DLT-413 new component ([#1144](#1144)) ([ad4c6c3](ad4c6c3)) * **Split Button:** DLT-3257 add show-divider prop ([#1173](#1173)) ([5bef411](5bef411)) * **Tabs:** DLT-3114 add vertical orientation variant ([#1120](#1120)) ([3290a23](3290a23)) * **Tabs:** DLT-3162 add spread prop for horizontal distribution ([#1135](#1135)) ([6e7cd04](6e7cd04)) * **Text:** export text component from dialtone-vue ([757fdb7](757fdb7)) * **Tokens:** DLT-2937 introduce spacing and layout token systems ([#1045](#1045)) ([de23e12](de23e12)) * **Tokens:** update color ramps and consumer color migration tools DLT-3004 DLT-3005 DLT-3006 DLT-3007 DLT-3008 ([#1054](#1054)) ([28cb1de](28cb1de))
# [9.180.0-next.1](dialtone/v9.179.0...dialtone/v9.180.0-next.1) (2026-04-15) ### Bug Fixes * **Doc:** NO-JIRA correct font size utility json ([#1015](#1015)) ([76cf0fa](76cf0fa)) * **Editor:** NO-JIRA remove extra blank line in editor.mdx ([18c47ef](18c47ef)) * possible merge issue in sidebarItem.vue ([5e2e91b](5e2e91b)) * **Split Button, Filter Pill:** DLT-3260 DLT-3261 correct misalignment at size 100 ([#1176](#1176)) ([fe2b777](fe2b777)) * **Stylelint:** disable hue-degree-notation rule and update OKLCH color syntax ([81f886e](81f886e)) * **Stylelint:** disable hue-degree-notation rule and update OKLCH color syntax ([8721f43](8721f43)) * **Tabs:** DLT-3251 improve accessibility ([#1171](#1171)) ([01b101e](01b101e)) * **Tokens:** DLT-3053 preserve root font size variable as reference ([#1093](#1093)) ([112ab4b](112ab4b)) * **Tokens:** handle multi-layer box shadows correctly in postcss plugin ([feea7f1](feea7f1)) * **Tokens:** NO-JIRA output line-height as unitless instead of percentages ([#1040](#1040)) ([33d8505](33d8505)) * **Tokens:** wrap css math expressions with calc() in layered build ([#966](#966)) ([1635f61](1635f61)) * update @dialpad/i18n dependencies to latest versions ([7a4b777](7a4b777)) ### Code Refactoring * **Combobox Multi Select:** NO-JIRA use logical css properties ([#1104](#1104)) ([a675daf](a675daf)) * **Components,recipes:** DLT-3043 DLT-3046 logical start/end naming with deprecated backward-compatible fallbacks ([#1079](#1079)) ([1d26a4e](1d26a4e)) * **Components:** DLT-3100 remove rootClass references ([#1184](#1184)) ([6531783](6531783)) * **Components:** dlt-3282 renamed show prop to open ([#1190](#1190)) ([5509d0f](5509d0f)) * **Components:** dlt-3283 refactor prop/slots to be consistent ([#1196](#1196)) ([2d22c76](2d22c76)) * **Css:** DLT-1876 css logical properties ([#872](#872)) ([1acd6c9](1acd6c9)) * **Modal:** DLT-3262 migrate DtModal to native dialog element ([#1179](#1179)) ([589edb5](589edb5)) * **Notice,banner,toast:** DLT-3296 decouple CSS and fix specificity violations ([#1194](#1194)) ([9579ac0](9579ac0)) * **Style:** DLT-3014 wrap all styles in css cascade layers ([#1061](#1061)) ([f8b3fed](f8b3fed)) * **Text:** DLT-2883 rename headline sizes from xxl => 2xl, etc ([#1029](#1029)) ([0ac86f3](0ac86f3)) * **Text:** DLT-3302 tone prop uses css variables instead of css utilities ([#1197](#1197)) ([75df16f](75df16f)) * **Tokens,css,link,text,breadcrumbs,input Group,split Button:** tokens, CSS foundation, deprecations, tooling DLT-3011 DLT-2961 DLT-3068 DLT-3069 DLT-3070 DLT-3071 DLT-3072 ([#1092](#1092)) ([7783090](7783090)) * **Tokens:** DLT-2884 deprecate space tokens in favor of size tokens ([#1013](#1013)) ([63f6d2f](63f6d2f)) * **Tokens:** DLT-3013 convert color system from HSL to OKLCH ([#1060](#1060)) ([42dc418](42dc418)) ### Documentation * **Components:** DLT-3178 DLT-3182 unify code examples into single code-example component ([#1147](#1147)) ([d0b6e14](d0b6e14)) * **Css Utilities:** DLT-3295 DLT-3280 mark physical utilities as deprecated ([#1193](#1193)) ([73f1479](73f1479)) * **Dialtone Vue:** DLT-3226 update documentation and consuming code to numeric size scale ([#1158](#1158)) ([149c763](149c763)) * DLT-1510 DLT-1511 DLT-1512 site redesign and ia ([#968](#968)) ([6ea7eb9](6ea7eb9)) * DLT-2908 ui kits landing page ([#1049](#1049)) ([d805b9d](d805b9d)) * DLT-3243 fenced code blocks for examples and snippets ([#1166](#1166)) ([cb169df](cb169df)) * DLT-3250 display component status badges in sidebar and page headers ([#1170](#1170)) ([7d5f5d5](7d5f5d5)) * DLT-3252 add GFM alert syntax for notices ([#1172](#1172)) ([1697911](1697911)) * DLT-3258 DLT-3259 improve sidebar search with logical keywords and fuzzy matching ([#1174](#1174)) ([2bf4639](2bf4639)) * NO-JIRA fix focus left-nav items on keypress down up ([#1106](#1106)) ([882c9b9](882c9b9)) * **Playground:** DLT-3119 refine playground UI and integrate across doc site ([#1127](#1127)) ([ba6b599](ba6b599)) * **Playground:** DLT-3242 segmented control for compact enum props ([#1165](#1165)) ([fa440d8](fa440d8)) * **Utilities:** DLT-3183 convert to vue demo fenced block ([#1167](#1167)) ([522197f](522197f)) ### Features * **Avatar:** DLT-2942 updated avatar component ([#1047](#1047)) ([e50563d](e50563d)) * **Avatar:** DLT-3161 change from clickable to "interactive" ([#1182](#1182)) ([2d00fe0](2d00fe0)) * **Banner, Notice, Toast, Breadcrumbs, Checkbox, Radio, Input, Select Menu, Empty State:** visual refresh, dt-text integration, form api parity DLT-2944 DLT-2945 DLT-2949 DLT-2959 DLT-2960 DLT-2957 ([#1091](#1091)) ([16e908a](16e908a)) * **Button,tabs,split Button:** visual refresh and slot updates DLT-2946 DLT-2947 DLT-2965 ([#1090](#1090)) ([ffdd56f](ffdd56f)) * **Components:** DLT-3157 align critical/positive prop values and CSS class names ([#1177](#1177)) ([edd98b2](edd98b2)) * **Components:** DLT-3159 enforce positive boolean props ([#1198](#1198)) ([ac73bdc](ac73bdc)) * **Css Utility:** DLT-2715 add default color for border css utilities ([#874](#874)) ([71310ab](71310ab)) * **Dialtone Css, Dialtone Tokens, Dialtone Vue:** DLT-3197 DLT-3198 DLT-3199 DLT-3200 DLT-3201 DLT-3202 DLT-3203 DLT-3204 DLT-3205 DLT-3206 DLT-3207 DLT-3208 DLT-3209 DLT-3210 DLT-3211 token-stop utility classes, migration tooling, and documentation ([#1150](#1150)) ([6237b84](6237b84)) * **Dialtone Vue:** DLT-3225 migrate component size props to numeric ordinal scale ([#1157](#1157)) ([725bd41](725bd41)) * **Eslint Plugin Dialtone:** DLT-3047 physical-to-logical naming migration tooling ([#1163](#1163)) ([d94dc4b](d94dc4b)) * **Eslint Plugin Dialtone:** DLT-3227 DLT-3228 add deprecated-tshirt-sizes ESLint rule and migration codemod ([#1159](#1159)) ([21a011f](21a011f)) * **Filter Pill:** DLT-2704 create component ([#473](#473)) ([6267445](6267445)) * **Filter Pill:** DLT-2941 DtFilterPill improvements ([#1110](#1110)) ([23c5689](23c5689)) * **Focusgroup:** DLT-3285 add v-dt-focusgroup directive for declarative roving tabindex ([#1187](#1187)) ([e1ea074](e1ea074)) * **Focustrap:** DLT-3297 add v-dt-focustrap directive for declarative focus trapping ([#1195](#1195)) ([6ef9a21](6ef9a21)) * **Link,button:** DLT-3012 add underline prop ([#1059](#1059)) ([1cb91ea](1cb91ea)) * **Mode Directive, Mode Island:** DLT-3067 DLT-3078 DLT-3077 add `v-dt-mode` directive ([#1109](#1109)) ([f97efca](f97efca)) * **Segmented Control:** DLT-413 new component ([#1144](#1144)) ([ad4c6c3](ad4c6c3)) * **Split Button:** DLT-3257 add show-divider prop ([#1173](#1173)) ([5bef411](5bef411)) * **Style:** DLT-2976 update underline thickness and offset ([#1058](#1058)) ([1e56947](1e56947)) * **Tabs:** DLT-3114 add vertical orientation variant ([#1120](#1120)) ([3290a23](3290a23)) * **Tabs:** DLT-3162 add spread prop for horizontal distribution ([#1135](#1135)) ([6e7cd04](6e7cd04)) * **Text:** export text component from dialtone-vue ([757fdb7](757fdb7)) * **Text:** new vue component DLT-2864 DLT-2863 DLT-2862 ([#995](#995)) ([196ef8a](196ef8a)) * **Tokens:** DLT-1977 add high contrast design tokens and theme ([#938](#938)) ([a1c0457](a1c0457)) * **Tokens:** DLT-2767 exploratory shell themes ([#909](#909)) ([3afcf63](3afcf63)) * **Tokens:** DLT-2781 create color-assistive themes ([#921](#921)) ([a6cc397](a6cc397)) * **Tokens:** DLT-2802 DLT-2563 layered theming system and mode island component ([#945](#945)) ([5c72c6c](5c72c6c)) * **Tokens:** DLT-2937 introduce spacing and layout token systems ([#1045](#1045)) ([de23e12](de23e12)) * **Tokens:** update color ramps and consumer color migration tools DLT-3004 DLT-3005 DLT-3006 DLT-3007 DLT-3008 ([#1054](#1054)) ([28cb1de](28cb1de)) * **Typography, Tokens:** DLT-2856 DLT-2857 next typography system ([#994](#994)) ([d744e97](d744e97))
3 participants
Remove rootClass references
🛠️ Type Of Change
These types will increment the version number on release:
📖 Jira Ticket
This is just the first part of the ticket.
https://dialpad.atlassian.net/browse/DLT-3100
📖 Description
Updated
rootClassto be onlyclasson components:Breaking Changes
New Props (not breaking, but worth noting)
Before:
After:
Side note: I also removed
addClassStyleAttrs($attrs)since seems like it is for Vue2 compat mode which is not needed anymore.It is still present on some
recipes, if this get approves I will remove it from there too.There is still
inputWrapperClassoninput.vueandwrapperClassonListItemThey are for a nested wrapper element, not the root but ... stil confusing..
on
input.vuewe still haveinputClassandinputWrapperClass. At least we don't haverootClassanymore. My goodnessRefactors Vue components to remove custom class props (rootClass/containerClass/wrapperClass) in favor of standard class/$attrs; removes addClassStyleAttrs, updates removeClassStyleAttrs, and adjusts templates, tests, stories, and variants across many components.