feat(combobox-with-popover, combobox-multi-select): DLT-3395 forward dialogClass to popover dialog - #1254
Merged
Josh Everhart (jeverhart-dialpad) merged 5 commits intoMay 7, 2026
Conversation
…dialogClass to popover dialog
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited) Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (2)
Adds a dialogClass prop to DtComboboxWithPopover and DtComboboxMultiSelect, forwarding it to DtPopover for scoped styling; includes Storybook argTypes updates and unit tests. Overall Judgement: ✅ Ready to merge Rationale: Small, backward-compatible API addition with tests and docs that follows existing component patterns. WalkthroughAdds a new ChangesDialog Class Customization
sequenceDiagram
participant Consumer
participant DtComboboxMultiSelect
participant DtComboboxWithPopover
participant DtPopoverDialog as DtPopover(dialog)
Consumer->>DtComboboxMultiSelect: pass prop dialogClass="custom-dialog-class"
DtComboboxMultiSelect->>DtComboboxWithPopover: forward :dialog-class="dialogClass"
DtComboboxWithPopover->>DtPopoverDialog: bind :dialog-class="dialogClass"
DtPopoverDialog-->>document.body: teleport dialog element with class "custom-dialog-class"
Suggested reviewers:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Josh Everhart (jeverhart-dialpad)
marked this pull request as ready for review
May 5, 2026 16:52
![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: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/dialtone-vue/components/combobox_multi_select/combobox_multi_select.test.js`:
- Around line 376-379: The test currently finds the popover dialog via the
internal ref ('content') on DtPopover
(wrapper.findComponent(DtPopover).findComponent({ ref: 'content' })) which
couples the spec to implementation; change the test to target a stable data-qa
attribute on the popover dialog instead (e.g., find the element with
[data-qa="combobox-popover"] under DtPopover) and assert that element exists and
has the 'custom-dialog-class'; if the dialog element in the component doesn’t
yet expose a data-qa attribute, add one there (on the popover/dialog root) so
tests can select it reliably.
- Around line 369-373: The beforeEach is remounting a new wrapper via
_setWrappers() without unmounting any previously mounted wrapper, which can
leave stale DOM; fix by ensuring any existing wrapper is unmounted before
remounting — either call wrapper.unmount() (or wrapper.destroy()) at the start
of this beforeEach or add/ensure an afterEach that unmounts the wrapper;
reference the _setWrappers function and the wrapper/input variables so the
teardown runs before creating a new instance.
In
`@packages/dialtone-vue/components/combobox_with_popover/combobox_with_popover.test.js`:
- Around line 560-563: Replace the brittle ref-based traversal that looks up the
popover content via findComponent(DtPopover).findComponent({ ref: 'content' })
with a data-qa selector lookup (e.g.
findComponent(DtPopover).find('[data-qa="combobox-popover-content"]') or
wrapper.find('[data-qa="combobox-popover-content"]')); assert existence and
classes against that element and ensure the popover renders the matching data-qa
attribute in the template so the test follows the repository's data-qa selector
convention.
- Around line 554-557: The nested beforeEach is mounting a new component on top
of an existing one; call wrapper?.unmount() (or wrapper.destroy()/await
wrapper.unmount() depending on your test utils) before invoking _mountWrapper()
so the previous instance is properly torn down; update the block around
_mountWrapper and _openComboboxPopover to unmount the existing wrapper variable
first to avoid stacked instances.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: f1692322-d0cf-452d-8347-28a0f1af13fa
📒 Files selected for processing (8)
packages/dialtone-vue/components/combobox_multi_select/combobox_multi_select.stories.jspackages/dialtone-vue/components/combobox_multi_select/combobox_multi_select.test.jspackages/dialtone-vue/components/combobox_multi_select/combobox_multi_select.vuepackages/dialtone-vue/components/combobox_multi_select/combobox_multi_select_default.story.vuepackages/dialtone-vue/components/combobox_with_popover/combobox_with_popover.stories.jspackages/dialtone-vue/components/combobox_with_popover/combobox_with_popover.test.jspackages/dialtone-vue/components/combobox_with_popover/combobox_with_popover.vuepackages/dialtone-vue/components/combobox_with_popover/combobox_with_popover_default.story.vue
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/dialtone-vue/components/combobox_multi_select/combobox_multi_select.test.js`:
- Around line 375-380: The test "should apply the class to the popover dialog
element" contains two assertions; split it into two separate tests so each
assertion stands alone: one test that queries
document.body.querySelector('[data-qa="dt-popover"]') and asserts the dialog is
not null, and a second test that queries the same selector and asserts
dialog.classList.contains('custom-dialog-class') is true; update the spec names
to reflect each behavior (e.g., "renders the popover dialog" and "applies the
custom dialog class") and keep the same selector and test setup used in the
original it block.
In
`@packages/dialtone-vue/components/combobox_with_popover/combobox_with_popover.test.js`:
- Around line 560-566: The test "should apply the class to the popover dialog
element" contains two assertions; split it into two separate it blocks so each
test has a single assertion: one test should query document.body for
'[data-qa="dt-popover"]' and assert the dialog is not null, and a second test
(with a clear name like "applies custom dialog class") should assert
dialog.classList.contains('custom-dialog-class') === true; update the existing
it block into two it(...) calls and reuse the same query logic to locate the
popover element.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: fe903ddb-9593-44b3-807f-2c4eb566faee
📒 Files selected for processing (2)
packages/dialtone-vue/components/combobox_multi_select/combobox_multi_select.test.jspackages/dialtone-vue/components/combobox_with_popover/combobox_with_popover.test.js
Contributor
|
Please add either the |
Contributor
|
✔️ Deploy previews ready! |
Josh Everhart (jeverhart-dialpad)
requested a review
from Bianca Artola (bianca-artola-dialpad)
May 6, 2026 17:51
Brad Paugh (braddialpad)
approved these changes
May 6, 2026
Brad Paugh (braddialpad)
left a comment
Brad Paugh (braddialpad)
left a comment
Contributor
There was a problem hiding this comment.
Looks good, thanks
Josh Everhart (jeverhart-dialpad)
added
the
no-visual-test
Belu Montoya (belumontoya)
approved these changes
May 7, 2026
Josh Everhart (jeverhart-dialpad)
deleted the
DLT-3395-combobox-dialog-class
branch
May 7, 2026 16:50
Brad Paugh (braddialpad)
pushed a commit
that referenced
this pull request
May 12, 2026
# [3.220.0](dialtone-vue/v3.219.4...dialtone-vue/v3.220.0) (2026-05-12) ### Features * **Combobox With Popover, Combobox Multi Select:** DLT-3395 forward dialogClass to popover dialog ([#1254](#1254)) ([0059451](0059451))
Brad Paugh (braddialpad)
pushed a commit
that referenced
this pull request
May 12, 2026
# [9.183.0](dialtone/v9.182.3...dialtone/v9.183.0) (2026-05-12) ### Features * **Combobox With Popover, Combobox Multi Select:** DLT-3395 forward dialogClass to popover dialog ([#1254](#1254)) ([0059451](0059451))
6 tasks
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
3 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.
feat(combobox-with-popover, combobox-multi-select): DLT-3395 forward dialogClass to popover dialog
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
https://dialpad.atlassian.net/browse/DLT-3395
📖 Description
Adds a
dialogClassprop toDtComboboxWithPopoverandDtComboboxMultiSelect. The prop forwards through to the underlying<dt-popover>'s existingdialogClass, which lands on thed-popover__dialogelement.Forwarding chain:
DtComboboxMultiSelect → DtComboboxWithPopover → DtPopover → d-popover__dialog.Matches the established forwarding pattern used by
DtHovercard,DtModal, andDtBanner. Tested locally on the docs site and also via storybook (https://dialtone.dialpad.com/vue/deploy-previews/pr-1254/?path=/story/components-combobox-multi-select--default&args=dialogClass:test-class).Usage:
💡 Context
DtPopoveralready exposes adialogClassprop (popover.vue:58), but neither combobox component forwarded it to consumers. This blocked any CSS customization scoped to a specific combobox usage — including overriding intrinsic styles like min-height on the popover header.The
dialogClass+ descendant selector approach was chosen over per-element wrapper-class props (e.g. a separate headerWrapperClass) because it provides broader reach with a smaller API surface, and matches the convention already used elsewhere in Dialtone.📝 Checklist
For all PRs:
For all Vue changes:
🔮 Next Steps
📷 Screenshots / GIFs
Context of where this may be useful: aligning tabs in the header of a combobox popover:
🔗 Sources