docs(extension-test): NO-JIRA add dt-migration-helper extension test page - #1331
docs(extension-test): NO-JIRA add dt-migration-helper extension test page#1331Belu Montoya (belumontoya) wants to merge 4 commits into
Conversation
|
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. WalkthroughAdds Changesdt-migration-helper Extension Test Page
Suggested Reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsThese MCP integrations need to be re-authenticated in the Integrations settings: Sentry Comment |
There was a problem hiding this comment.
💡 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".
| <dt-avatar :size="size" full-name="Test User" /> | ||
| <dt-text kind="code" :size="100">{{ size }}</dt-text> | ||
| </div> | ||
| <dt-avatar-group> |
There was a problem hiding this comment.
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 👍 / 👎.
| <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> |
There was a problem hiding this comment.
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 👍 / 👎.
… invalid raised surface
…ities, add all missing components
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)
apps/dialtone-documentation/docs/guides/migration/extension-test/index.md (1)
1-5:⚠️ Potential issue | 🟠 MajorAdd page to site navigation or set
noSidebar: trueif 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: trueto the front matter (if this is truly a preview-only fixture that should remain hidden), or- Add an entry to
_data/site-nav.jsonunder 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
📒 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)
|
✔️ Deploy previews ready! |
|
close without merge? revisit? |
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.mdSections:
Not intended to be merged — preview-only test fixture.