docs(modal): NO-JIRA fix code examples#1207
docs(modal): NO-JIRA fix code examples#1207Josh Everhart (jeverhart-dialpad) merged 6 commits intostagingfrom
Conversation
WalkthroughAdds a new public prop Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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 `@apps/dialtone-documentation/docs/components/modal.md`:
- Line 58: Update the guidance sentence about aria-labelledby: replace the
grammatically incorrect phrase to say "The value of `aria-labelledby` should
match the heading element’s `id`" and ensure it references the modal root
element and the heading (e.g., `h2`) so it's clear the root element's
`aria-labelledby` value must equal the heading element's `id`.
- Around line 321-329: Update the displayed vueCode snippet for the "Custom
Header and Content" example so it matches the live example: place the trigger
dt-button before the dt-modal, include the modal-class="d-mt0" attribute on the
dt-modal, and ensure the same bindings (:show="isOpen" and
`@update`:show="updateShow") are present; modify the vueCode string/markdown that
renders this snippet to reflect those exact tags and attributes (look for the
vueCode variable or snippet block that contains dt-button and dt-modal).
🪄 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: f2cffbfb-93a7-4fa6-abf6-3048e7bc659a
📒 Files selected for processing (2)
apps/dialtone-documentation/docs/.vuepress/exampleComponents/ExampleModal.vueapps/dialtone-documentation/docs/components/modal.md
| <dt-button | ||
| @click="openModal" | ||
| > | ||
| Click to open | ||
| </dt-button> | ||
| <dt-modal | ||
| :show="isOpen" | ||
| modal-class="d-mt0" | ||
| @update:show="updateShow" |
There was a problem hiding this comment.
Update the displayed vueCode snippet to match the live example block.
The live "Custom Header and Content" example now has the trigger before <dt-modal> and uses modal-class="d-mt0", but the shown snippet still omits both.
🔧 Proposed patch
<code-example-tabs
vueCode='
+<dt-button
+ `@click`="openModal"
+>
+ Click to open
+</dt-button>
<dt-modal
:show="isOpen"
+ modal-class="d-mt0"
`@update`:show="updateShow"
>As per coding guidelines: “For ‘Custom Header and Content’ examples, place the trigger button before the dt-modal in the snippet shown … and include modal-class="d-mt0" on the dt-modal snippet where spacing-top needs to be corrected.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dialtone-documentation/docs/components/modal.md` around lines 321 - 329,
Update the displayed vueCode snippet for the "Custom Header and Content" example
so it matches the live example: place the trigger dt-button before the dt-modal,
include the modal-class="d-mt0" attribute on the dt-modal, and ensure the same
bindings (:show="isOpen" and `@update`:show="updateShow") are present; modify the
vueCode string/markdown that renders this snippet to reflect those exact tags
and attributes (look for the vueCode variable or snippet block that contains
dt-button and dt-modal).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/dialtone-documentation/docs/.vuepress/exampleComponents/ExampleModal.vue`:
- Around line 68-71: The change flips the shared prop default for
fixedHeaderFooter in ExampleModal.vue from false to true, unintentionally
affecting all uses; revert the default back to false in the ExampleModal
component's prop definition (fixedHeaderFooter) and instead set
fixedHeaderFooter explicitly to true only at the example call sites that require
a fixed header/footer so only those instances change behavior.
🪄 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: 1d7fdcfa-1d4d-460c-85a7-8fe814819ac9
📒 Files selected for processing (2)
apps/dialtone-documentation/docs/.vuepress/exampleComponents/ExampleModal.vueapps/dialtone-documentation/docs/components/modal.md
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/dialtone-documentation/docs/components/modal.md
| fixedHeaderFooter: { | ||
| type: Boolean, | ||
| default: false, | ||
| default: true, | ||
| }, |
There was a problem hiding this comment.
Confirm the fixedHeaderFooter default flip is intentional.
Line 70 changes a shared default from false to true, which affects every ExampleModal usage that omits this prop. If only specific examples need fixed header/footer, keep the old default and set it explicitly at those call sites.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/dialtone-documentation/docs/.vuepress/exampleComponents/ExampleModal.vue`
around lines 68 - 71, The change flips the shared prop default for
fixedHeaderFooter in ExampleModal.vue from false to true, unintentionally
affecting all uses; revert the default back to false in the ExampleModal
component's prop definition (fixedHeaderFooter) and instead set
fixedHeaderFooter explicitly to true only at the example call sites that require
a fixed header/footer so only those instances change behavior.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/dialtone-documentation/docs/.vuepress/exampleComponents/ExampleModal.vue (1)
47-50:initiallyOpenname doesn’t match runtime behavior.At Line 84 the prop opens initially, but at Line 91-Line 93 it is always closed on mount. If this is only for snapshot/HTML generation, consider a more explicit prop name to avoid misuse in future examples.
♻️ Suggested clarity refactor
- initiallyOpen: { + renderOpenMarkupOnce: { type: Boolean, default: false, }, ... - isOpen: this.initiallyOpen, + isOpen: this.renderOpenMarkupOnce, ... - if (this.initiallyOpen) { + if (this.renderOpenMarkupOnce) { this.isOpen = false; }Also applies to: 84-94
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dialtone-documentation/docs/.vuepress/exampleComponents/ExampleModal.vue` around lines 47 - 50, The prop initiallyOpen is misleading because runtime code on mount forces the modal closed; either make the prop actually control initial open state or rename it to reflect the special-case behavior. Fix by renaming the prop (e.g., initiallyOpen -> openOnStaticRender or openForHtmlSnapshot) and update all references in the component: the prop declaration, template bindings, any places reading props (mounted/created hooks that force-close the modal), and any tests/examples; alternatively, if you prefer semantics, remove the mount code that forces close so initiallyOpen truly opens on mount (update mounted hook logic in the component methods that currently override the prop, and adjust related state initialization in the component).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/dialtone-documentation/docs/.vuepress/exampleComponents/ExampleModal.vue`:
- Around line 47-50: The prop initiallyOpen is misleading because runtime code
on mount forces the modal closed; either make the prop actually control initial
open state or rename it to reflect the special-case behavior. Fix by renaming
the prop (e.g., initiallyOpen -> openOnStaticRender or openForHtmlSnapshot) and
update all references in the component: the prop declaration, template bindings,
any places reading props (mounted/created hooks that force-close the modal), and
any tests/examples; alternatively, if you prefer semantics, remove the mount
code that forces close so initiallyOpen truly opens on mount (update mounted
hook logic in the component methods that currently override the prop, and adjust
related state initialization in the component).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 1a7e102a-efb3-47cb-b9c6-a8e9ef65e1dc
📒 Files selected for processing (1)
apps/dialtone-documentation/docs/.vuepress/exampleComponents/ExampleModal.vue
|
✔️ Deploy previews ready! |
Brad Paugh (braddialpad)
left a comment
There was a problem hiding this comment.
Thanks!
# [9.180.0](dialtone/v9.179.0...dialtone/v9.180.0) (2026-04-17) ### Bug Fixes * **Rich Text Editor:** DP-179947 use URL as display text when inserting link without text ([#1202](#1202)) ([30f2504](30f2504)) ### Documentation * **Modal:** NO-JIRA fix code examples ([#1207](#1207)) ([aedd4a1](aedd4a1)) ### Features * **Eslint Plugin Dialtone:** DLT-3314 add deprecated-dialtone-component rule ([#1200](#1200)) ([c73b7ee](c73b7ee))
docs(modal): NO-JIRA code example update
Obligatory GIF (super important!)
🛠️ Type Of Change
These types will increment the version number on release:
These types will not increment the version number, but will still deploy to documentation site on release:
📖 Jira Ticket
📖 Description
This PR aligns the code examples on the modal documentation page with UI examples shown on the page. The modal page now also auto-generates the HTML examples to reduce manual updates to that in the future.
Items addressed
Button order and text
Code example accuracy
:kind="secondaryButtonKind"from code examples:showFooter="true"from the Fixed Header example:bannerKind(camelCase) tobanner-kind(kebab-case) in the Has Banner examplekindnot being forwarded to<dt-modal>in ExampleModal (was only passed to the confirm button)ExampleModal improvements
initiallyOpenprop andmounted()hook to support the HTML capture patternfixedHeaderFooterprop (DtModal handles its own default)class="d-mt0"to the trigger button to prevent unwanted spacing from thed-stack8layout contextAccessibility and grammar
idvalue" → "should match theidof its heading element"vueCodeexampleNote about revised HTML examples for modal
In order to get the HTML automatically, an extra example modal was added for each entry inside of a hidden
<div>set toinertto capture the proper HTML markup when the page loads. It renders an<example-modal initially-open />so thatDtLazyShow(which only initializes the modal DOM when show=true) actually renders the full modal markup. The inert attribute prevents that hidden open modal from trapping focus or intercepting keyboard/pointer events on the page. Thed-h0 d-of-hidden d-vi-hidden d-pe-noneclasses keep it visually hidden and out of the layout. Not sure if this is overkill, but it does seem to work.💡 Context
As reported in a Dialtone channel, the code examples provided on the modal documentation page did not match what was actually being displayed -- the buttons were in the wrong order.
📝 Checklist
For all PRs:
🔮 Next Steps
There are a few margin fixes in this version because the modal component doesn't append to the body or an ancestor by default. This is something we may want to revisit. In the example of the docs page, it is grouped with a button inside of a stack, and so either the button or the modal end up inheriting a margin.
Another item to consider is whether we should use a stack in the example instead of margins on the buttons. Not sure if that was a purposeful decision.
📷 Screenshots / GIFs
Before
After
🔗 Sources
Corrects modal docs: fixes example button order, adds inert wrapper divs and an initiallyOpen prop to render full modal markup without trapping focus, and auto-generates HTML examples from Vue refs to reduce manual updates; also tightens copy and standardizes button styling.