refactor(components): dlt-3282 renamed show prop to open#1190
refactor(components): dlt-3282 renamed show prop to open#1190Ignacio Ropolo (iropolo) wants to merge 2 commits intonextfrom
Conversation
WalkthroughThis pull request standardizes the visibility control API across Modal, Tooltip, Toast, Popover, and related components by renaming the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~20 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 |
|
Please add either the |
|
✔️ Deploy previews ready! |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/dialtone-docs/src/content/reference/reference-component-api-patterns.md (1)
78-97:⚠️ Potential issue | 🟡 MinorUpdate the remaining Tooltip warning text to
openfor consistency.This section correctly standardizes visibility to
open, but the same page still says DtTooltip warns onenabled + show. Please rename that mention toenabled + opento avoid mixed guidance.Suggested doc fix
-- DtTooltip: warns when both `enabled` and `show` props are set +- DtTooltip: warns when both `enabled` and `open` props are setBased on learnings: Component documentation must include Vue source, tests, Storybook stories, component docs JSON, VuePress documentation, and MCP server data — all must stay in sync.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/dialtone-docs/src/content/reference/reference-component-api-patterns.md` around lines 78 - 97, Locate the remaining tooltip warning that says "enabled + show" and update it to "enabled + open" for consistency with the visibility prop naming; specifically, search for the DtTooltip mention in the reference-component-api-patterns content and replace the wording (and any example or explanatory text) to use "open" instead of "show" so it matches the `update:open` convention and the table/listing of emits.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@packages/dialtone-docs/src/content/reference/reference-component-api-patterns.md`:
- Around line 78-97: Locate the remaining tooltip warning that says "enabled +
show" and update it to "enabled + open" for consistency with the visibility prop
naming; specifically, search for the DtTooltip mention in the
reference-component-api-patterns content and replace the wording (and any
example or explanatory text) to use "open" instead of "show" so it matches the
`update:open` convention and the table/listing of emits.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: c2347579-2bd2-4b94-8aa2-4c63a9aeb099
📒 Files selected for processing (27)
.claude/rules/vue-components.md.claude/skills/vue-conventions.mdCLAUDE.mdapps/dialtone-documentation/docs/.vuepress/exampleComponents/ExampleModal.vueapps/dialtone-documentation/docs/.vuepress/exampleComponents/ExampleToast.vueapps/dialtone-documentation/docs/.vuepress/views/IconCatalog.vueapps/dialtone-documentation/docs/components/modal.mdapps/dialtone-documentation/docs/components/toast.mdapps/dialtone-documentation/docs/scratch.mdpackages/dialtone-docs/src/content/reference/reference-component-api-patterns.mdpackages/dialtone-vue/components/modal/modal.stories.jspackages/dialtone-vue/components/modal/modal.test.jspackages/dialtone-vue/components/modal/modal.vuepackages/dialtone-vue/components/modal/modal_default.story.vuepackages/dialtone-vue/components/popover/popover_variants.story.vuepackages/dialtone-vue/components/toast/toast.stories.jspackages/dialtone-vue/components/toast/toast.test.jspackages/dialtone-vue/components/toast/toast.vuepackages/dialtone-vue/components/toast/toast_default.story.vuepackages/dialtone-vue/components/tooltip/tooltip.stories.jspackages/dialtone-vue/components/tooltip/tooltip.test.jspackages/dialtone-vue/components/tooltip/tooltip.vuepackages/dialtone-vue/components/tooltip/tooltip_change_on_click.story.vuepackages/dialtone-vue/components/tooltip/tooltip_default.story.vuepackages/dialtone-vue/components/tooltip/tooltip_flip.story.vuepackages/dialtone-vue/components/tooltip/tooltip_variants.story.vuepackages/dialtone-vue/recipes/buttons/callbar_button/callbar_button.vue
Brad Paugh (braddialpad)
left a comment
There was a problem hiding this comment.
We are fixing some inconsistencies in prop naming and this show vs open will be always open.
Components updated:
Modal
Tooltip
Toast <-- doubts on this but still updated it.
Doubt:dt-lazy-show , this uses show, but it is semantically different from overlay open/close. I didn't update this.
lol hate to say this.. but does it make sense to use "show" instead of "open" for everything then? I think it kinda works for everything while open doesn't.
Heh. Did a little research... The web platform strongly favors
Top-tier Vue libraries:
Non-Vue design systems:
The trend is clearly toward open:
Bottom line
|
Renamed show prop to open
🛠️ Type Of Change
These types will increment the version number on release:
📖 Jira Ticket
https://dialpad.atlassian.net/browse/DLT-3282
📖 Description
We are fixing some inconsistencies in prop naming and this
showvsopenwill be alwaysopen.Components updated:
Doubt:
show, but it is semantically different from overlay open/close. I didn't update this.Breaking changes
DtModal
DtTooltip
DtToast
Migration Examples:
Refactors Modal, Tooltip, and Toast components by renaming the
showprop/event toopenfor consistent prop naming across overlay components. Updates include component implementations, tests, stories, and documentation. Breaking API changes for all three components.