Skip to content

docs(extension-test): NO-JIRA add dt-migration-helper extension test page - #1331

Open
Belu Montoya (belumontoya) wants to merge 4 commits into
nextfrom
docs/extension-test-page
Open

docs(extension-test): NO-JIRA add dt-migration-helper extension test page#1331
Belu Montoya (belumontoya) wants to merge 4 commits into
nextfrom
docs/extension-test-page

Conversation

@belumontoya

Copy link
Copy Markdown
Contributor

Visual test fixture for the dt-migration-helper Chrome extension. Exercises all three toggles: Inspect, Apply stable tokens, Apply stable structure.

Page: guides/migration/extension-test/index.md

Sections:

  • Tokens — base color palette (all families × stops), semantic surfaces, semantic backgrounds, semantic foregrounds, semantic borders, spacing, shadows, border radius, typography
  • Utility Classes — d-fc-, d-bgc-, d-bc-* (semantic + base color stops), scrollbar directive
  • Components — DtChip (span.d-chip), DtModal (dialog.d-modal), DtAvatar, DtBadge, DtButton, DtLink, DtBox, DtText, DtBanner, DtStack, DtInput, DtSelectMenu, DtNotice, DtToast

Not intended to be merged — preview-only test fixture.

@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Adds a Vue-based visual fixture for the dt-migration-helper Chrome extension, covering token palettes/semantics, utility color classes (d-fc/d-bgc/d-bc) plus scrollbar, and demo-only Dialtone components (e.g., chips, modal, forms, toasts) to exercise all extension toggles.

Overall Judgement: 🚫 Needs rework — The fixture is explicitly preview-only and intended for testing, not for merging into the main codebase.

Walkthrough

Adds apps/dialtone-documentation/docs/guides/migration/extension-test/index.md, a new 664-line Vue-based documentation page serving as a visual test fixture for the dt-migration-helper extension. It renders design token grids, utility class demos, a scrollbar directive example, and comprehensive component showcases, driven by a <script setup> block.

Changes

dt-migration-helper Extension Test Page

Layer / File(s) Summary
Page metadata and script data
apps/dialtone-documentation/docs/guides/migration/extension-test/index.md
Front matter sets page title and description; <script setup> defines isOpen reactive state and constant arrays for color families/stops, semantic groups, typography, spacing, and avatar sizes.
Tokens section
apps/dialtone-documentation/docs/guides/migration/extension-test/index.md
Demo-only blocks render v-for-driven grids for base color palette, semantic surface/background/foreground/border colors, spacing, shadows, border radius, and typography variants.
Utility classes and scrollbar directive
apps/dialtone-documentation/docs/guides/migration/extension-test/index.md
Demo blocks cover d-fc-*, d-bgc-*, d-bc-* class families including base colors, semantic groups, and black stops, plus a v-dt-scrollbar:always directive example rendering sample lines in a scrollable container.
Component demos
apps/dialtone-documentation/docs/guides/migration/extension-test/index.md
Adds representative variant and state examples for chip, modal (wired to isOpen), avatar/avatar group, badge, banner, buttons/split button/button group, links/breadcrumbs, box/card/text/stack, input/input group, select menu, checkbox/radio/toggle, segmented control, filter pill, presence, loader/skeleton, progress circle, collapsible, dropdown, notice, toast, tab group/tab panels, pagination, keyboard shortcut, empty state, and codeblock.

Suggested Reviewers

  • braddialpad
  • ninarepetto
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs/extension-test-page

Warning

Review ran into problems

🔥 Problems

These MCP integrations need to be re-authenticated in the Integrations settings: Sentry


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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 16d5a88038

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".

Comment thread apps/dialtone-documentation/docs/guides/migration/extension-test/index.md Outdated
<dt-avatar :size="size" full-name="Test User" />
<dt-text kind="code" :size="100">{{ size }}</dt-text>
</div>
<dt-avatar-group>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Replace the nonexistent DtAvatarGroup tag

dt-avatar-group is not a Dialtone component or CSS hook in this repo: a repo-wide search for DtAvatarGroup, avatar-group, and d-avatar-group only finds this new page, and packages/dialtone-vue/components/Avatar exports only DtAvatar. Vue will leave this as an unknown custom element, so the fixture does not exercise an actual avatar group; use an existing supported layout/CSS pattern or add/export the component before referencing it.

Useful? React with 👍 / 👎.

Comment on lines +223 to +224
<div v-dt-scrollbar:always class="d-hmx-200 d-of-y-auto d-baw1 d-bas-solid d-bc-default d-p-300 d-bar-200">
<p v-for="n in 15" :key="n">Line {{ n }} — v-dt-scrollbar:always</p>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Wrap scrollbar content in a single viewport child

The scrollbar directive expects exactly one child as the viewport: packages/dialtone-vue/directives/scrollbar_directive/scrollbar.js passes el.children[0] to OverlayScrollbars, and the public scrollbar docs call out that single-child constraint. Here the v-for creates fifteen sibling <p> elements, so only the first paragraph becomes the viewport and the overflow/scrollbar behavior being tested is not representative; wrap the repeated paragraphs in a single child container as the existing docs do.

Useful? React with 👍 / 👎.

Comment thread apps/dialtone-documentation/docs/guides/migration/extension-test/index.md Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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/extension-test/index.md (1)

1-5: ⚠️ Potential issue | 🟠 Major

Add page to site navigation or set noSidebar: true if preview-only.

The front matter lacks noSidebar: true, and the page is not registered in _data/site-nav.json. Per the coding guidelines, new documentation pages must be added to site navigation. Either:

  • Add noSidebar: true to the front matter (if this is truly a preview-only fixture that should remain hidden), or
  • Add an entry to _data/site-nav.json under the migration guides section (if this page should be discoverable).

The PR description states this is "not intended to be merged," but the file is being permanently added. Clarify the intent: does this land as a navigable guide, or should it be hidden from the documentation site?

🤖 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 `@apps/dialtone-documentation/docs/guides/migration/extension-test/index.md`
around lines 1 - 5, The front matter for this documentation page lacks
navigation metadata, creating ambiguity about whether it should be discoverable.
Determine the intent of this page: if it is a preview-only fixture that should
remain hidden from the documentation site, add `noSidebar: true` to the front
matter section at the top of the file; alternatively, if this page should be
included as a navigable guide, add an appropriate entry to the site navigation
configuration file under the migration guides section. The front matter
currently shows only title, description, and closing delimiters, so the
noSidebar property would be inserted alongside the existing metadata if that is
the chosen approach.

Sources: Coding guidelines, Learnings

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

Outside diff comments:
In `@apps/dialtone-documentation/docs/guides/migration/extension-test/index.md`:
- Around line 1-5: The front matter for this documentation page lacks navigation
metadata, creating ambiguity about whether it should be discoverable. Determine
the intent of this page: if it is a preview-only fixture that should remain
hidden from the documentation site, add `noSidebar: true` to the front matter
section at the top of the file; alternatively, if this page should be included
as a navigable guide, add an appropriate entry to the site navigation
configuration file under the migration guides section. The front matter
currently shows only title, description, and closing delimiters, so the
noSidebar property would be inserted alongside the existing metadata if that is
the chosen approach.

ℹ️ Review info
⚙️ Run configuration

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

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: a188cfd1-8f06-4802-b328-c0f061c38e57

📥 Commits

Reviewing files that changed from the base of the PR and between e05b877 and a5c446b.

📒 Files selected for processing (1)
  • apps/dialtone-documentation/docs/guides/migration/extension-test/index.md
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • dialpad/ios (manual)
  • dialpad/firespotter (manual)

@github-actions

Copy link
Copy Markdown
Contributor

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

@francisrupert

Copy link
Copy Markdown
Contributor

close without merge? revisit?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants