Skip to content

refactor(components): DLT-3284 refactor props to not have native html attributes names #1204

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Ignacio Ropolo (iropolo) merged 3 commits into from
Apr 20, 2026
Merged

refactor(components): DLT-3284 refactor props to not have native html attributes names #1204

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b56daa3
refactor(components): DLT-3284 refactor props to not have native html…
Apr 16, 2026
b938c8d
added missing argtypes
Apr 16, 2026
4d397e2
missing argtypes entry and default standarization to undefined value
Apr 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/combinator/src/components/renderer/renderer_target.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ function renderError (exception, container) {
render(h(DtNotice, {
kind: 'critical',
showClose: false,
title: ERROR_MESSAGE,
headerText: ERROR_MESSAGE,
}, {
default: () => exception.toString(),
}), container);
Expand Down
4 changes: 2 additions & 2 deletions packages/combinator/src/variants/variants_banner.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* eslint-disable max-len */


export default {
default: {
slots: {
Expand All @@ -8,7 +8,7 @@ export default {
},
},
props: {
title: {
headerText: {
initialValue: 'Example banner',
},
kind: {
Expand Down
6 changes: 3 additions & 3 deletions packages/combinator/src/variants/variants_modal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
/* eslint-disable max-len */

export default {
default: {
props: {
title: {
headerText: {
initialValue: 'Example title',
},
copy: {
Expand All @@ -18,7 +18,7 @@ export default {
},
danger: {
props: {
title: {
headerText: {
initialValue: 'Example title',
},
copy: {
Expand Down
10 changes: 5 additions & 5 deletions packages/combinator/src/variants/variants_notice.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@


export default {
default: {
slots: {
Expand All @@ -8,15 +8,15 @@ export default {
},
},
props: {
title: {
headerText: {
initialValue: 'Base title',
},
},
},

'info with action and hide close': {
props: {
title: { initialValue: 'Info title' },
headerText: { initialValue: 'Info title' },
kind: { initialValue: 'info' },
showClose: { initialValue: false },
},
Expand All @@ -28,7 +28,7 @@ export default {

'important warning with no message': {
props: {
title: { initialValue: 'Warning title' },
headerText: { initialValue: 'Warning title' },
kind: { initialValue: 'warning' },
important: { initialValue: true },
},
Expand Down
8 changes: 4 additions & 4 deletions packages/combinator/src/variants/variants_toast.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@


export default {
default: {
Expand All @@ -8,7 +8,7 @@ export default {
},
},
props: {
title: {
headerText: {
initialValue: 'Base title (optional)',
},
show: {
Expand All @@ -19,7 +19,7 @@ export default {

'info with action and hide close': {
props: {
title: { initialValue: 'Info title' },
headerText: { initialValue: 'Info title' },
kind: { initialValue: 'info' },
showClose: { initialValue: false },
show: { initialValue: true },
Expand All @@ -32,7 +32,7 @@ export default {

'important warning with no message': {
props: {
title: { initialValue: 'Warning title' },
headerText: { initialValue: 'Warning title' },
kind: { initialValue: 'warning' },
important: { initialValue: true },
show: { initialValue: true },
Expand Down
6 changes: 3 additions & 3 deletions packages/dialtone-vue/components/banner/banner.stories.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export const argTypesData = {
disable: true,
},
},
titleId: {
headerId: {
table: {
defaultValue: {
summary: 'generated unique ID',
Expand Down Expand Up @@ -130,7 +130,7 @@ export const Default = {
render: Template,

args: {
title: 'Optional title',
headerText: 'Optional title',
action: 'Action',
kind: 'base',
},
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Expand Down Expand Up @@ -181,7 +181,7 @@ export const CustomBackground = {
...Default.args,
backgroundImage,
backgroundSize: 'contain',
title: '',
headerText: '',
action: '',
showIcon: false,
dialogClass: 'd-fc-neutral-white',
Expand Down
24 changes: 12 additions & 12 deletions packages/dialtone-vue/components/banner/banner.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,25 +9,25 @@
class="d-banner__dialog"
:class="dialogClass"
:role="role"
:aria-labelledby="titleId"
:aria-labelledby="headerId"
:aria-describedby="contentId"
>
<dt-notice-icon
v-if="showIcon"
:kind="kind"
:class="{ 'd-notice__icon--has-title': title || $slots.title }"
:class="{ 'd-notice__icon--has-title': headerText || $slots.header }"
>
<!-- @slot Slot for custom icon -->
<slot name="icon" />
</dt-notice-icon>
<dt-notice-content
:title-id="titleId"
:header-id="headerId"
:content-id="contentId"
:title="title"
:header-text="headerText"
>
<template #title>
<!-- @slot Slot for the title -->
<slot name="title" />
<template #header>
<!-- @slot Slot for the header -->
<slot name="header" />
</template>
Comment on lines +24 to 31
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 16, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add a BREAKING CHANGE: footer for this public API rename.

Lines 24–31 and 74–92 rename public props/slot (title/titleId/#title → headerText/headerId/#header). Please ensure the PR/commit includes an explicit BREAKING CHANGE: footer with migration mapping; otherwise this can ship as a patch and silently break consumers.

As per coding guidelines, “This is a public npm design system library … Flag any prop/event/slot removal or rename without BREAKING CHANGE footer.”

Also applies to: 74-92

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/dialtone-vue/components/banner/banner.vue` around lines 24 - 31,
This PR renames public props/slot on the Banner component (title → headerText,
titleId → headerId, slot `#title` → `#header`) and must include a BREAKING CHANGE
footer with a clear migration map; add a single-line "BREAKING CHANGE: ..."
footer to the commit/PR description and update the changelog/release notes with
an explicit mapping (title → headerText, titleId → headerId, `#title` → `#header`)
and guidance for consumers to rename usages accordingly so this won't ship as a
silent patch.

<!-- @slot the main textual content of the banner -->
<slot />
Expand Down Expand Up @@ -68,10 +68,10 @@ export default {

props: {
/**
* Sets an ID on the title element of the component. Useful for aria-describedby
* or aria-labelledby or any other reason you may need an id to refer to the title.
* Sets an ID on the header element of the component. Useful for aria-describedby
* or aria-labelledby or any other reason you may need an id to refer to the header.
*/
titleId: {
headerId: {
type: String,
default () { return utils.getUniqueString(); },
},
Expand All @@ -86,9 +86,9 @@ export default {
},

/**
* Title header of the notice. This can be left blank to remove the title from the notice entirely.
* Header text of the banner. This can be left blank to remove the header from the banner entirely.
*/
title: {
headerText: {
type: String,
default: '',
},
Expand Down
4 changes: 2 additions & 2 deletions packages/dialtone-vue/components/banner/banner_default.story.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
<dt-banner
v-show="displayBanner"
:kind="$attrs.kind"
:title="$attrs.title"
:title-id="$attrs.titleId"
:header-text="$attrs.headerText"
:header-id="$attrs.headerId"
:content-id="$attrs.contentId"
:important="$attrs.important"
:pinned="$attrs.pinned"
Expand Down
4 changes: 2 additions & 2 deletions packages/dialtone-vue/components/modal/modal.stories.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ viverra iaculis. Interdum et malesuada fames ac ante ipsum primis in faucibus. V
maximus ipsum ex. Curabitur elementum luctus augue, quis eleifend tortor feugiat vel. \
Maecenas maximus, ipsum et laoreet congue, diam massa aliquam libero, at pellentesque \
orci ipsum et velit.`,
title: 'Example Title',
headerText: 'Example Title',
Comment thread
coderabbitai[bot] marked this conversation as resolved.
onClose: action('update:open'),
};

Expand Down Expand Up @@ -183,7 +183,7 @@ export const WithBanner = {
render: DefaultTemplate,

args: {
bannerTitle: 'Example banner',
bannerHeaderText: 'Example banner',
},

parameters: { ...Default.parameters },
Expand Down
22 changes: 11 additions & 11 deletions packages/dialtone-vue/components/modal/modal.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@ import { mount } from '@vue/test-utils';
const SYNC_EVENT_NAME = 'update:open';

const MOCK_MODAL_COPY = 'test modal copy';
const MOCK_MODAL_TITLE = 'test modal title';
const MOCK_MODAL_HEADER_TEXT = 'test modal header text';
const MOCK_MODAL_BANNER = 'test modal banner';
const MOCK_MODAL_DEFAULT_SLOT = 'test content';
const MOCK_MODAL_HEADER_SLOT = 'test header';
const MOCK_MODAL_BANNER_SLOT = 'title';

const baseProps = {
title: MOCK_MODAL_TITLE,
headerText: MOCK_MODAL_HEADER_TEXT,
copy: MOCK_MODAL_COPY,
bannerTitle: MOCK_MODAL_BANNER,
bannerHeaderText: MOCK_MODAL_BANNER,
open: true,
};

Expand Down Expand Up @@ -81,9 +81,9 @@ describe('DtModal Tests', () => {
expect(overlay.element.tagName).toBe('DIALOG');
});

it('should render the title content', () => {
it('should render the header text content', () => {
expect(title.exists()).toBe(true);
expect(title.text()).toEqual(MOCK_MODAL_TITLE);
expect(title.text()).toEqual(MOCK_MODAL_HEADER_TEXT);
});

it('should render the banner content', () => {
Expand Down Expand Up @@ -129,7 +129,7 @@ describe('DtModal Tests', () => {
});

describe('When slots are provided', () => {
it('Should display slotted header instead of title', () => {
it('Should display slotted header instead of headerText', () => {
mockSlots = {
header: MOCK_MODAL_HEADER_SLOT,
};
Expand All @@ -139,7 +139,7 @@ describe('DtModal Tests', () => {
expect(title.text()).toEqual(MOCK_MODAL_HEADER_SLOT);
});

it('Should display slotted banner instead of bannerTitle', () => {
it('Should display slotted banner instead of bannerHeaderText', () => {
mockSlots = {
banner: MOCK_MODAL_BANNER_SLOT,
};
Expand Down Expand Up @@ -219,11 +219,11 @@ describe('DtModal Tests', () => {

it('Should apply banner class', async () => {
const bannerClass = 'banner-class';
const bannerTitle = 'title';
const bannerHeaderText = 'title';

await wrapper.setProps({
open: true,
bannerTitle,
bannerHeaderText,
bannerClass,
});

Expand All @@ -236,7 +236,7 @@ describe('DtModal Tests', () => {
await wrapper.setProps({
open: true,
bannerKind: 'info',
bannerTitle: 'title',
bannerHeaderText: 'title',
});

banner = wrapper.find('[data-qa="dt-modal-banner"]');
Expand All @@ -248,7 +248,7 @@ describe('DtModal Tests', () => {
await wrapper.setProps({
show: true,
bannerKind: 'critical',
bannerTitle: 'title',
bannerHeaderText: 'title',
});

banner = wrapper.find('[data-qa="dt-modal-banner"]');
Expand Down
18 changes: 9 additions & 9 deletions packages/dialtone-vue/components/modal/modal.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,17 @@
@keydown="onKeydown"
>
<div
v-if="open && (hasSlotContent($slots.banner) || bannerTitle)"
v-if="open && (hasSlotContent($slots.banner) || bannerHeaderText)"
data-qa="dt-modal-banner"
:class="[
'd-modal__banner',
bannerClass,
bannerKindClass,
]"
>
<!-- @slot Slot for the banner, defaults to bannerTitle prop -->
<!-- @slot Slot for the banner, defaults to bannerHeaderText prop -->
<slot name="banner">
{{ bannerTitle }}
{{ bannerHeaderText }}
</slot>
</div>
<transition
Expand All @@ -54,7 +54,7 @@
class="d-modal__header"
data-qa="dt-modal-title"
>
<!-- @slot Slot for dialog header section, taking the place of any "title" text prop -->
<!-- @slot Slot for dialog header section, taking the place of any "headerText" text prop -->
<slot name="header" />
</div>
<dt-text
Expand All @@ -69,7 +69,7 @@
class="d-modal__header"
data-qa="dt-modal-title"
>
{{ title }}
{{ headerText }}
</dt-text>
<div
v-if="hasSlotContent($slots.default)"
Expand Down Expand Up @@ -201,17 +201,17 @@ export default {
},

/**
* Title text to display in the modal header.
* Header text to display in the modal header.
*/
title: {
headerText: {
type: String,
default: '',
},

/**
* Title text to display in the modal banner.
* Header text to display in the modal banner.
*/
bannerTitle: {
bannerHeaderText: {
type: String,
default: '',
},
Comment on lines +204 to 217
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 16, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Mark this API rename with a BREAKING CHANGE: footer before merge.

Line 204 and Line 214 rename public modal props (title → headerText, bannerTitle → bannerHeaderText). In this public npm package, that must be explicitly marked as breaking to prevent silent patch-level consumer breakage.

As per coding guidelines, **/*: "This is a public npm library. Breaking changes without BREAKING CHANGE footer ship as patches and silently break consumers."

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/dialtone-vue/components/modal/modal.vue` around lines 204 - 217,
This change renames public props and must be documented as a breaking change:
add a BREAKING CHANGE footer to the commit/PR description stating the old and
new prop names (e.g. BREAKING CHANGE: prop `title` renamed to `headerText`; prop
`bannerTitle` renamed to `bannerHeaderText`) so consumers are warned; reference
the modal component props (headerText, bannerHeaderText) in the message and
include migration guidance (old → new) before merging.

Expand Down
4 changes: 2 additions & 2 deletions packages/dialtone-vue/components/modal/modal_default.story.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
<template>
<div>
<dt-modal
:title="$attrs.title"
:banner-title="$attrs.bannerTitle"
:header-text="$attrs.headerText"
:banner-header-text="$attrs.bannerHeaderText"
:open="isOpen"
:kind="$attrs.kind"
:size="$attrs.size"
Expand Down
Loading
Loading