feat(scrollbar): DLT-3158 implement object props

[Brad Paugh (braddialpad)]

Obligatory GIF (super important!)

🛠️ Type Of Change

• Feature

📖 Jira Ticket

DLT-3158

📖 Description

Transitions v-dt-scrollbar to an object-based API and renames the confusing :never argument to :always.

Key changes:

• Object API: v-dt-scrollbar="{ showScrollbar: 'always', offset: { blockStart: '2rem', inlineEnd: '4rem' } }" — directive now accepts an optional config object with showScrollbar and offset fields. The existing arg shorthand (:always, :scroll, :move) is preserved.
• Offset support: New offset object on the directive sets CSS custom properties (--dt-scrollbar-offset-block-start/end, --dt-scrollbar-offset-inline-start/end) on the host element. Supports block/inline shorthands that expand to both sides. CSS in scrollbar.less uses these to reposition the scrollbar tracks via logical property overrides.
• :never → :always rename: The :never arg was unintuitive ("never auto-hide" = always visible). Renamed to :always. A migration script (dialtone-migrate-scrollbar-always) and migration guide are included.
• DtBox alignment: scrollbar="never" prop value updated to scrollbar="always".
• updated hook: Directive now reacts to binding changes after mount.
• Simplified internals: AUTO_HIDE_MAP, OFFSET_SIDES, and resolveOffset collapsed into a ternary + applyOffset helper.

💡 Context

The :never shorthand was confusing — it mapped to OverlayScrollbars' autoHide: 'never' (meaning always show), which reads backwards. The offset feature was requested to support use cases where fixed headers/footers overlap the scrollable region without requiring manual CSS overrides.

📦 Cross-Package Impact

Package	Changes	Downstream Impact
dialtone-css	New offset CSS custom property rules in scrollbar.less; new migration script	Vue directive depends on these custom properties
dialtone-vue	Updated directive (scrollbar.js), updated DtBox scrollbar constant	Documentation site demos updated
dialtone-documentation	Updated scrollbar.md (object API docs, new Offset variant), migration guide, site-nav.json	—

📄 Documentation Artifacts

Artifact	Status
Vue source (scrollbar.js, box.vue)	✅ Updated
Tests (scrollbar.test.js, box.test.js)	✅ Updated
Storybook story (scrollbar_directive_default.story.vue)	✅ Updated
VuePress docs (scrollbar.md, box.md)	✅ Updated
Migration guide (guides/migration/scrollbar-always/)	✅ Added
Migration script (dialtone-migrate-scrollbar-always)	✅ Added

📝 Checklist

For all PRs:

• I have ensured no private Dialpad links or info are in the code or pull request description (Dialtone is a public repo!).
• I have reviewed my changes.
• I have added all relevant documentation.
• I have considered the performance impact of my change.

For all Vue changes:

• I have added / updated unit tests.
• I have validated components with a screen reader.
• I have validated components keyboard navigation.

For all CSS changes:

• I have used design tokens whenever possible.
• I have considered how this change will behave on different screen sizes.
• I have visually validated my change in light and dark mode.
• I have used gap or flexbox properties for layout instead of margin whenever possible.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Renames scrollbar mode never → always, adds directive object syntax (showScrollbar, offset, blockClasses/inlineClasses), implements an updated hook to reconfigure OverlayScrollbars, exposes CSS offset variables and positioning rules, adds a migration CLI, and updates docs, examples, stories, tests, and package bin wiring.

Changes

Scrollbar :never → :always rename + offset support

Layer / File(s)	Summary
Constants / API surface packages/dialtone-vue/components/box/box_constants.js, packages/dialtone-vue/components/box/box.vue	Replaces 'never' with 'always' in allowed scrollbar values and JSDoc @values.
Directive core (behavior & offsets) packages/dialtone-vue/directives/scrollbar_directive/scrollbar.js	Adds resolveShowScrollbar(value, arg) mapping ('always'→'never', 'enter'→'leave'), computes numeric autoHideDelay (0 or 1300), applies offset CSS vars, supports blockClasses/inlineClasses, and adds an updated hook that calls os.options(...) and reapplies offsets/classes.
Directive tests packages/dialtone-vue/directives/scrollbar_directive/scrollbar.test.js	Refactors to factory-based mounts, expands overlayscrollbars mock (destroy, options, elements), and adds tests for arg/object showScrollbar, numeric autoHideDelay, offset CSS vars, and scrollbar-class application.
CSS positioning packages/dialtone-css/lib/build/less/components/scrollbar.less	Adds inset positioning using --dt-scrollbar-offset-{block-start,block-end,inline-start,inline-end} (with 0 fallbacks) and cornerless inset overrides for vertical/horizontal scrollbars.
Migration CLI packages/dialtone-css/lib/build/js/dialtone_migrate_scrollbar_always/index.mjs	New CLI that scans .vue/.html, transforms v-dt-scrollbar:never → v-dt-scrollbar:always and scrollbar="never" → scrollbar="always" (plus string-literal variants), supports --cwd, --dry-run, --yes, prompts, prints summaries, and exports transformContent(content).
Package wiring (bin) packages/dialtone-css/package.json	Registers dialtone-migrate-scrollbar-always binary pointing to the built CLI entry.
Documentation & migration guide apps/dialtone-documentation/docs/components/scrollbar.md, apps/dialtone-documentation/docs/guides/migration/scrollbar-always/index.md, apps/dialtone-documentation/docs/guides/migration/index.md, apps/dialtone-documentation/docs/_data/site-nav.json	Docs updated for Vue 3 directive examples, allowed modes (enter, always, scroll, move), object syntax (showScrollbar, offset, class hooks), offset docs/CSS hooks, migration guide page, checklist entry, and nav link.
Examples / templates / demos apps/dialtone-documentation/docs/**, apps/dialtone-documentation/docs/.vuepress/baseComponents/*, packages/combinator/src/components/code_panel/code_panel.vue, packages/dialtone-vue/directives/scrollbar_directive/scrollbar_directive_default.story.vue	Demo snippets, templates and stories updated from :never/scrollbar="never" to :always/scrollbar="always"; story adds offset demo blocks and label changes.
Box component tests packages/dialtone-vue/components/box/box.test.js	DtBox test updated to mount with scrollbar: 'always' while preserving existing assertions.
Minor build/style edits packages/dialtone-css/lib/build/less/components/box.less	Whitespace/blank-line adjustments only.

Sequence Diagram(s)

sequenceDiagram
participant Template
participant DtScrollbarDirective
participant OverlayScrollbars
participant DOM
Template->>DtScrollbarDirective: mount/update with v-dt-scrollbar (arg or { showScrollbar, offset })
DtScrollbarDirective->>DtScrollbarDirective: resolveShowScrollbar(binding)
DtScrollbarDirective->>OverlayScrollbars: init or update autoHide + autoHideDelay via os.options(...)
DtScrollbarDirective->>DOM: set/remove --dt-scrollbar-offset-* CSS vars and apply classes
OverlayScrollbars->>DOM: render scrollbar elements (.os-scrollbar-*)
DOM-->>Template: visible scrollbar behavior

Loading

Suggested labels

visual-test-ready

Suggested reviewers

• francisrupert
• iropolo
• ninarepetto

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch DLT-3158

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

• Generate code and open pull requests
• Plan features and break down work
• Investigate incidents and troubleshoot customer tickets together
• Automate recurring tasks and respond to alerts with triggers
• Summarize progress and report instantly

Built for teams:

• Shared memory across your entire org—no repeating context
• Per-thread sandboxes to safely plan and execute work
• Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bf05a42d34

ℹ️ 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".

[coderabbitai]

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

apps/dialtone-documentation/docs/guides/migration/index.md (1)

44-44: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix duplicated migration numbering in the checklist tables.

Line 44 restarts at 15 after adding component item 15 on Line 38; this should be incremented (e.g., 16) to keep the sequence unambiguous.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/dialtone-documentation/docs/guides/migration/index.md` at line 44, The
migration checklist has a duplicated item number: the table row currently
starting with "| 15 | [Vue 2 Removal](./vue2-removal/) | **Yes** | — | Vue 2
support dropped. Last Vue 2 version: `9.154.0`. |" should be renumbered to
continue the sequence (change the leading "15" to "16") so the checklist
numbering remains unique and sequential; update the same table row in
docs/guides/migration/index.md accordingly.

🤖 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/guides/migration/index.md`:
- Around line 38-39: Coordinate downstream adoption: ensure the migration for
renaming v-dt-scrollbar:never → v-dt-scrollbar:always (and DtBox
scrollbar="never" → scrollbar="always") is scheduled and applied in the linked
repo dialpad/firespotter before promoting the release; create and run the
dialtone-migrate-scrollbar-always command against the affected files (e.g.,
ai_csat_category_rank.vue, ai_csat_call_details_explanation.vue) and confirm
CI/tests pass, then document the change and rollout plan so downstream teams are
aware.

In `@apps/dialtone-documentation/docs/guides/migration/scrollbar-always/index.md`:
- Around line 19-22: Update the migration guide table to also document the
Vue-bound form of the prop (e.g., :scrollbar="'never'" and
:scrollbar="'always'") in addition to the literal attributes and directive
forms; specifically add entries mapping v-dt-scrollbar:never →
v-dt-scrollbar:always and scrollbar="never" / scrollbar="always" →
:scrollbar="'never'" / :scrollbar="'always'" (or similar phrasing) so bound
consumers see the supported migration path, and apply the same additions to the
other occurrence block that mirrors lines 32-42.
- Around line 19-22: The guide uses the internal name "leave" for auto-hide but
the public API/docs use "enter"; update the wording to use the public scrollbar
mode names consistently—replace any occurrences of "leave" with "enter" and
ensure examples and the table (e.g. `v-dt-scrollbar:never`,
`v-dt-scrollbar:always`, `scrollbar="never"`, `scrollbar="always"`) and
surrounding text refer to the public modes (`enter`, `never`, `always`,
`scroll`, `move`) so the DtBox scrollbar values and other directive arguments
remain described with the public vocabulary.

In `@packages/dialtone-vue/directives/scrollbar_directive/scrollbar.test.js`:
- Around line 11-30: The test uses an ID selector '#viewport' which is brittle;
update the test fixture and lookup to use a data-qa attribute instead. Change
the WrapperDefault markup to include a data-qa attribute on the viewport element
(e.g., <div data-qa="viewport">) and update the mountWith helper to set
viewportElement = wrapper.find('[data-qa="viewport"]').element; adjust any other
references to the old ID (viewportElement, WrapperDefault) so they use the
data-qa selector while keeping the same mountWith and DtScrollbarDirective
usage.
- Around line 143-165: The two tests under the "when directive value has
block/inline shorthand offset" describe combine two assertions each; split them
so each it contains a single assertion (or convert to parameterized tests with
it.each). Specifically, for the checks inside the existing it('should set
block-start and block-end from block shorthand') replace it with two separate
tests asserting only '--dt-scrollbar-offset-block-start' and only
'--dt-scrollbar-offset-block-end' (referencing wrapper, mountWith, makeWrapper,
and the v-dt-scrollbar directive), and do the same for the inline shorthand test
so there are individual tests for '--dt-scrollbar-offset-inline-start' and
'--dt-scrollbar-offset-inline-end'.
- Around line 63-125: The mounted hook is passing autoHideDelay as a string via
a template literal; change the mounted implementation (mounted hook where
autoHideDelay is set) to use a number: autoHideDelay: noDelay ? 0 : 1300 (remove
the backtick/template literal) and update the tests in scrollbar.test.js that
assert OverlayScrollbars was called to expect a numeric autoHideDelay (0 or
1300) instead of a string; additionally, consider refactoring assertions in the
tests (which reference OverlayScrollbars constructor calls) to verify observable
DOM/CSS behavior (class presence, data attributes or CSS variables/offsets)
rather than tightly coupling to the mock payload structure.

---

Outside diff comments:
In `@apps/dialtone-documentation/docs/guides/migration/index.md`:
- Line 44: The migration checklist has a duplicated item number: the table row
currently starting with "| 15 | [Vue 2 Removal](./vue2-removal/) | **Yes** | — |
Vue 2 support dropped. Last Vue 2 version: `9.154.0`. |" should be renumbered to
continue the sequence (change the leading "15" to "16") so the checklist
numbering remains unique and sequential; update the same table row in
docs/guides/migration/index.md accordingly.

🪄 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: a74dbecb-ec7a-46e8-a68e-fb4bb5318513

📥 Commits

Reviewing files that changed from the base of the PR and between 30a5ea2 and bf05a42.

📒 Files selected for processing (24)

• apps/dialtone-documentation/docs/.vuepress/baseComponents/UiKitsComparisonTable.vue
• apps/dialtone-documentation/docs/.vuepress/baseComponents/tokens/TokenTable.vue
• apps/dialtone-documentation/docs/_data/site-nav.json
• apps/dialtone-documentation/docs/components/box.md
• apps/dialtone-documentation/docs/components/card.md
• apps/dialtone-documentation/docs/components/scrollbar.md
• apps/dialtone-documentation/docs/dialtone/whats-new/posts/2026-4-22.md
• apps/dialtone-documentation/docs/guides/migration/index.md
• apps/dialtone-documentation/docs/guides/migration/scrollbar-always/index.md
• apps/dialtone-documentation/docs/scratch.md
• apps/dialtone-documentation/docs/utilities/sizing/height.md
• apps/dialtone-documentation/docs/utilities/sizing/size.md
• apps/dialtone-documentation/docs/utilities/sizing/width.md
• packages/combinator/src/components/code_panel/code_panel.vue
• packages/dialtone-css/lib/build/js/dialtone_migrate_scrollbar_always/index.mjs
• packages/dialtone-css/lib/build/less/components/box.less
• packages/dialtone-css/lib/build/less/components/scrollbar.less
• packages/dialtone-css/package.json
• packages/dialtone-vue/components/box/box.test.js
• packages/dialtone-vue/components/box/box.vue
• packages/dialtone-vue/components/box/box_constants.js
• packages/dialtone-vue/directives/scrollbar_directive/scrollbar.js
• packages/dialtone-vue/directives/scrollbar_directive/scrollbar.test.js
• packages/dialtone-vue/directives/scrollbar_directive/scrollbar_directive_default.story.vue

[coderabbitai]

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
`@packages/dialtone-vue/directives/scrollbar_directive/scrollbar_directive_default.story.vue`:
- Line 27: Replace the hardcoded English story labels ("Always visible" and the
other two literal strings present in this story file) with FTL localization keys
and use the project's localization helper to render them (e.g., call the
translation helper used across stories like $t or the Storybook i18n wrapper).
Locate the three literal strings in scrollbar_directive_default.story.vue (the
visible story titles/labels) and change each to reference a new FTL key (add
keys to the appropriate .ftl file) and update the template to call the
translator with those keys so the story labels are localized.
- Line 30: The shorthand for the scrollbar directive was renamed from
v-dt-scrollbar:never to v-dt-scrollbar:always which is a breaking change for
consumers; add a backward-compatible migration layer that accepts
v-dt-scrollbar:never (map it to the new behavior or emit a clear
console/developer warning when used), update the directive registration/lookup
to recognize both tokens (v-dt-scrollbar:never and v-dt-scrollbar:always), and
ensure the release includes a BREAKING CHANGE footer and migration note so
downstream repos like dialpad/firespotter are notified.

🪄 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: c2cb5dfc-7d3a-406e-99cc-70b5fd289617

📥 Commits

Reviewing files that changed from the base of the PR and between bf05a42 and 8a1af0d.

📒 Files selected for processing (2)

• packages/dialtone-css/lib/build/less/components/scrollbar.less
• packages/dialtone-vue/directives/scrollbar_directive/scrollbar_directive_default.story.vue

[coderabbitai]

♻️ Duplicate comments (4)

packages/dialtone-vue/directives/scrollbar_directive/scrollbar.test.js (3)

74-77: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Split multi-assert tests into single-assert cases (or it.each).

These it blocks contain multiple expectations; please split them so each test has one assertion.

As per coding guidelines, “One assertion per test. Use beforeAll/beforeEach to reduce repetition. Use it.each for similar tests with different values.”

Also applies to: 156-159, 187-195

🤖 Prompt for 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.

In `@packages/dialtone-vue/directives/scrollbar_directive/scrollbar.test.js`
around lines 74 - 77, The test "should add d-scrollbar class and data attribute"
bundles two assertions; split it into two separate tests so each `it` has a
single expectation: one `it` that asserts
wrapper.element.classList.contains('d-scrollbar') and another `it` that asserts
wrapper.element.getAttribute('data-overlayscrollbars-initialize') === 'true'.
Apply the same refactor to the other multi-assert blocks referenced (lines
~156-159 and ~187-195) — extract each expectation into its own `it` or use
`it.each` for similar assertions, and reuse the existing `wrapper` setup via
beforeEach to avoid duplication.

---

67-71: 🧹 Nitpick | 🔵 Trivial | 🏗️ Heavy lift

Reduce constructor-payload assertions; prioritize observable directive behavior.

These cases are asserting internal mock payloads as the primary signal. Prefer behavior-level checks (DOM attributes/classes/CSS vars and binding update effects) so refactors of integration plumbing don’t cause brittle failures.

As per coding guidelines, “Flag tests that assert on implementation details (mock call counts) instead of behavior (rendered output, emitted events).”

Also applies to: 90-95, 103-108, 116-121, 132-137

🤖 Prompt for 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.

In `@packages/dialtone-vue/directives/scrollbar_directive/scrollbar.test.js`
around lines 67 - 71, The test currently asserts internal constructor payloads
for OverlayScrollbars (expect(OverlayScrollbars).toHaveBeenCalledWith(...))
which couples tests to implementation; instead update tests (places referencing
OverlayScrollbars calls around the 'should initialize...' cases and similar
blocks at lines ~90, ~103, ~116, ~132) to verify observable directive behavior:
check DOM mutations on wrapper.element and viewportElement (expected classes,
inline styles/CSS variables, ARIA attributes), and verify directive binding
updates trigger expected DOM changes or emitted events, removing direct
assertions on OverlayScrollbars call arguments and call counts.

---

11-12: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Replace #viewport fixtures/selectors with data-qa hooks.

Using id="viewport" and wrapper.find('#viewport') makes these tests diverge from repo test conventions. Please switch fixtures and queries to [data-qa="viewport"] consistently.

As per coding guidelines, “Select elements only via data-qa attributes (avoid CSS-class or bare-tag selectors).”

Also applies to: 24-30, 87-87, 100-100, 113-113, 127-127, 143-143, 165-165, 182-182

🤖 Prompt for 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.

In `@packages/dialtone-vue/directives/scrollbar_directive/scrollbar.test.js`
around lines 11 - 12, Tests use id="viewport" and selector '#viewport' in the
fixture created by WrapperDefault (via makeWrapper(`<div v-dt-scrollbar><div
id="viewport"></div></div>`)) and in subsequent wrapper.find calls; change the
fixture to use data-qa="viewport" and update all queries (e.g.,
wrapper.find('#viewport')) to wrapper.find('[data-qa="viewport"]') across the
test file (including the other occurrences noted) so tests follow the repo
convention of selecting by data-qa hooks.

packages/dialtone-vue/directives/scrollbar_directive/scrollbar.js (1)

39-51: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use a numeric autoHideDelay in mounted to match updated.

At Line 50, autoHideDelay is passed as a string, while Line 66 passes a number. Keep this numeric in both paths to avoid type drift against OverlayScrollbars config.

Proposed fix

         }, {
           scrollbars: {
             autoHide,
             clickScroll: true,
-            autoHideDelay: `${noDelay ? 0 : 1300}`,
+            autoHideDelay: noDelay ? 0 : 1300,
           },
         });

OverlayScrollbars v2.10.0 docs: what is the expected type for `scrollbars.autoHideDelay`?

🤖 Prompt for 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.

In `@packages/dialtone-vue/directives/scrollbar_directive/scrollbar.js` around
lines 39 - 51, In the mounted hook where OverlayScrollbars is created (the call
using OverlayScrollbars({ target: el, elements: { viewport: el.children[0] } },
{ scrollbars: { autoHide, clickScroll: true, autoHideDelay: `${noDelay ? 0 :
1300}`, }, ... }) ), change autoHideDelay from a string to a numeric value to
match updated; compute noDelay via resolveShowScrollbar(binding.value,
binding.arg) and pass autoHideDelay: noDelay ? 0 : 1300 (number) so the
scrollbars.autoHideDelay config receives a number instead of a string.

🤖 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.

Duplicate comments:
In `@packages/dialtone-vue/directives/scrollbar_directive/scrollbar.js`:
- Around line 39-51: In the mounted hook where OverlayScrollbars is created (the
call using OverlayScrollbars({ target: el, elements: { viewport: el.children[0]
} }, { scrollbars: { autoHide, clickScroll: true, autoHideDelay: `${noDelay ? 0
: 1300}`, }, ... }) ), change autoHideDelay from a string to a numeric value to
match updated; compute noDelay via resolveShowScrollbar(binding.value,
binding.arg) and pass autoHideDelay: noDelay ? 0 : 1300 (number) so the
scrollbars.autoHideDelay config receives a number instead of a string.

In `@packages/dialtone-vue/directives/scrollbar_directive/scrollbar.test.js`:
- Around line 74-77: The test "should add d-scrollbar class and data attribute"
bundles two assertions; split it into two separate tests so each `it` has a
single expectation: one `it` that asserts
wrapper.element.classList.contains('d-scrollbar') and another `it` that asserts
wrapper.element.getAttribute('data-overlayscrollbars-initialize') === 'true'.
Apply the same refactor to the other multi-assert blocks referenced (lines
~156-159 and ~187-195) — extract each expectation into its own `it` or use
`it.each` for similar assertions, and reuse the existing `wrapper` setup via
beforeEach to avoid duplication.
- Around line 67-71: The test currently asserts internal constructor payloads
for OverlayScrollbars (expect(OverlayScrollbars).toHaveBeenCalledWith(...))
which couples tests to implementation; instead update tests (places referencing
OverlayScrollbars calls around the 'should initialize...' cases and similar
blocks at lines ~90, ~103, ~116, ~132) to verify observable directive behavior:
check DOM mutations on wrapper.element and viewportElement (expected classes,
inline styles/CSS variables, ARIA attributes), and verify directive binding
updates trigger expected DOM changes or emitted events, removing direct
assertions on OverlayScrollbars call arguments and call counts.
- Around line 11-12: Tests use id="viewport" and selector '#viewport' in the
fixture created by WrapperDefault (via makeWrapper(`<div v-dt-scrollbar><div
id="viewport"></div></div>`)) and in subsequent wrapper.find calls; change the
fixture to use data-qa="viewport" and update all queries (e.g.,
wrapper.find('#viewport')) to wrapper.find('[data-qa="viewport"]') across the
test file (including the other occurrences noted) so tests follow the repo
convention of selecting by data-qa hooks.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 043a3b0c-4e31-4c9e-922c-6da7b52d6203

📥 Commits

Reviewing files that changed from the base of the PR and between 8a1af0d and 1e96fe9.

📒 Files selected for processing (2)

• packages/dialtone-vue/directives/scrollbar_directive/scrollbar.js
• packages/dialtone-vue/directives/scrollbar_directive/scrollbar.test.js

[coderabbitai]

Actionable comments posted: 3

🤖 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-css/lib/build/js/dialtone_migrate_scrollbar_always/index.mjs`:
- Around line 44-51: Narrow the global attribute rewrites so they only affect
DtBox components: update the four .replace calls that currently match
scrollbar="never", scrollbar='never', :scrollbar="'never'", and
:scrollbar='"never"' so their regexes require the attribute to appear inside a
DtBox tag (match both kebab-case <dt-box ...> and PascalCase <DtBox ...>)
instead of anywhere in the file; modify the patterns used in those .replace
calls to assert a preceding opening DtBox tag (e.g., by matching
"<\s*(dt-box|DtBox)\b" then any attrs before the targeted scrollbar attribute)
while preserving each replacement string exactly.

In `@packages/dialtone-vue/directives/scrollbar_directive/scrollbar.js`:
- Around line 3-8: resolveShowScrollbar currently treats any unknown mode
(including legacy 'never') as valid; explicitly handle the deprecated alias by
mapping 'never' → 'always' before the existing mapping logic in the
resolveShowScrollbar function so the rename is enforced by code (optionally emit
a deprecation warning when mode === 'never'); keep the rest of the mapping
(always → 'never', enter → 'leave') unchanged.

In `@packages/dialtone-vue/directives/scrollbar_directive/scrollbar.test.js`:
- Around line 74-77: The test "should add d-scrollbar class and data attribute"
combines two assertions; split it into two separate tests so each test asserts
one thing: one it block that checks
wrapper.element.classList.contains('d-scrollbar') and another it block that
checks wrapper.element.getAttribute('data-overlayscrollbars-initialize') ===
'true'; reuse the existing wrapper setup via beforeEach (or beforeAll) to avoid
repetition and, if you have more similar checks later, consider using it.each to
parameterize the assertions.

🪄 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: 786c61c4-8469-4b8b-9802-6a414f0affe4

📥 Commits

Reviewing files that changed from the base of the PR and between 1e96fe9 and 93ff100.

📒 Files selected for processing (4)

• apps/dialtone-documentation/docs/guides/migration/scrollbar-always/index.md
• packages/dialtone-css/lib/build/js/dialtone_migrate_scrollbar_always/index.mjs
• packages/dialtone-vue/directives/scrollbar_directive/scrollbar.js
• packages/dialtone-vue/directives/scrollbar_directive/scrollbar.test.js

[coderabbitai]

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 `@apps/dialtone-documentation/docs/components/scrollbar.md`:
- Around line 249-261: The docs claim invalid showScrollbar values are logged
and fall back to 'enter', but the code path in resolveShowScrollbar (in
scrollbar.js) currently forwards unknown values to OverlayScrollbars; either
implement validation inside resolveShowScrollbar to check the value against the
allowed set ['enter','always','scroll','move'], log an informational message
when an invalid value is given, and return the default 'enter', or remove this
validation paragraph from the scrollbar.md docs and any related examples; locate
usages around the v-dt-scrollbar directive and update the implementation or docs
accordingly so they match.

In `@packages/dialtone-vue/directives/scrollbar_directive/scrollbar.js`:
- Around line 81-83: The unmounted hook currently calls
instances.get(el).destroy() which can throw if no instance exists; update the
unmounted(el) implementation to first retrieve the instance via
instances.get(el), check that the instance is non-null/defined, call
instance.destroy() only when present, and then remove the entry from the
instances map (instances.delete(el)) to avoid leaks; reference the unmounted
function and the instances map and the instance.destroy() method when making the
change.

In `@packages/dialtone-vue/directives/scrollbar_directive/scrollbar.test.js`:
- Around line 218-221: The test currently asserts two classes in one spec; split
into separate assertions using either two it blocks or an it.each table to
follow one-assertion-per-test; replace the existing it('should add blockClasses
to the vertical scrollbar element', ...) with an it.each([['d-w12'],
['d-bgc-purple-300']]) that runs a single expectation against
mocks.blockScrollbar.classList.contains(className) or create two distinct
it(...) specs each asserting mocks.blockScrollbar.classList.contains('d-w12')
and mocks.blockScrollbar.classList.contains('d-bgc-purple-300') respectively,
reusing the existing beforeEach setup.
- Around line 69-245: Missing tests for the directive's updated hook; add tests
that simulate reactive binding changes and assert reconfiguration. Mount the
directive via makeWrapper/v-dt-scrollbar with an initial opts (e.g.,
showScrollbar or offset or blockClasses/inlineClasses), then call
wrapper.setProps to change opts and await update; assert
OverlayScrollbars.options (or mocks.options) was called with new
scrollbars.autoHide/autoHideDelay, confirm element style custom properties
(--dt-scrollbar-offset-*) updated for numeric/string offsets, and verify class
diffs applied to mocks.blockScrollbar and mocks.inlineScrollbar; also ensure no
duplicate initializations and that existing instance's destroy is not called
during update. Use references: the directive's updated hook, OverlayScrollbars
(and its options method), wrapper.setProps, and
mocks.blockScrollbar/mocks.inlineScrollbar.

🪄 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: 568cbfb8-83e7-456a-b1f2-985ec7658694

📥 Commits

Reviewing files that changed from the base of the PR and between 93ff100 and 07188d2.

📒 Files selected for processing (3)

• apps/dialtone-documentation/docs/components/scrollbar.md
• packages/dialtone-vue/directives/scrollbar_directive/scrollbar.js
• packages/dialtone-vue/directives/scrollbar_directive/scrollbar.test.js

[github-actions]

Please add either the visual-test-ready or no-visual-test label to this PR depending on whether you want to run visual tests or not.
It is recommended to run visual tests if your PR changes any UI. ‼️

[Brad Paugh (braddialpad)]

All issues have been resolved, please have a final lookthrough, there were quite a number of changes.

[github-actions]

✔️ Deploy previews ready!
😎 Dialtone documentation preview: https://dialtone.dialpad.com/deploy-previews/pr-1249/
😎 Dialtone-vue preview: https://dialtone.dialpad.com/vue/deploy-previews/pr-1249/

[Belu Montoya (belumontoya)]

I tested the migration regex against five inputs. It behaves correctly on the first four. The fifth () gets skipped because [\s\S]*?> stops at the first >, but that's a genuinely rare case for a one-shot migration script.