feat: Dropdown, Select and ItemList#631
Conversation
Split dropdown rendering into reusable menu list and item content helpers, add trigger and row customization slots, and cover custom item semantics with Cypress tests. Refresh the dropdown stories and docs to reflect grouped menus, nested submenus, trigger patterns, and full-row escape hatches.
Modernize TabButtons with script setup, improved value mapping, and icon-only accessibility support. Add Cypress coverage for selection and click handling, and switch the docs demo to use TabButtons for preview/code toggles.
Replace the shared Button abstraction with native buttons while keeping Reka UI state management. This restores the intended TabButtons styling, keeps icon-only accessibility intact, and aligns the docs demo with the updated segmented control presentation.
Publish VitePress preview builds under /pr-preview/pr-<number>/, comment the preview URL on pull requests, and clean up previews when PRs close while preserving the main docs site on gh-pages.
|
🚀 VitePress preview is ready: |
|
Note
|
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly summarizes the main feature additions: new/refactored Dropdown, Select, and ItemList components with comprehensive API improvements. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
v1-release/selection-menu-api-spec
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 @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
docs/meta/Textarea.md (1)
69-75:⚠️ Potential issue | 🟡 MinorAdd description for the
update:modelValueevent.The
update:modelValueemit event has an empty description (Line 72). Since this file is auto-generated, add a JSDoc description to the corresponding emit definition in the source component's type definition file.Based on learnings: "Every public prop, emit, and slot should have a JSDoc description" applies to src/components/**/types.ts.
docs/meta/DatePicker.md (1)
9-129:⚠️ Potential issue | 🟠 MajorAdd missing JSDoc descriptions for DatePicker API.
The DatePicker documentation is missing descriptions for most of its public API:
- 12 props without descriptions:
modelValue,value,placement,format,variant,readonly,placeholder,inputClass,allowCustom,autoClose,disabled,clearable- All 3 slots without descriptions:
target,prefix,suffix- Both emit events without descriptions:
update:modelValue,changeSince this file is auto-generated, add JSDoc comments to the corresponding type definitions in the DatePicker component source file.
Based on learnings: "Every public prop, emit, and slot should have a JSDoc description" applies to src/components/**/types.ts.
docs/meta/Tabs.md (1)
9-56:⚠️ Potential issue | 🟡 MinorAdd missing descriptions for Tabs component API.
Missing JSDoc descriptions for:
modelValueprop (Line 30)update:modelValueemit event (Line 53)Since this file is auto-generated, add JSDoc comments to the corresponding type definitions in the Tabs component source file.
Based on learnings: "Every public prop, emit, and slot should have a JSDoc description" applies to src/components/**/types.ts.
docs/meta/TextInput.md (1)
82-88:⚠️ Potential issue | 🟡 MinorAdd description for the
update:modelValueevent.The
update:modelValueemit event lacks a description (Line 85). Since this file is auto-generated, add a JSDoc description to the corresponding emit definition in the TextInput component's type definition file.Based on learnings: "Every public prop, emit, and slot should have a JSDoc description" applies to src/components/**/types.ts.
🧹 Nitpick comments (15)
docs/components/Docs/Demo.vue (1)
20-26: Nit: drop emptyclass=""attributes.Lines 21 and 26 have
class=""which is a no-op; safe to remove to reduce markup noise.♻️ Proposed cleanup
<div class="grid not-prose"> - <div class=""> - <TabButtons :buttons="previewTabs" v-model="activeTab" /> - </div> + <TabButtons :buttons="previewTabs" v-model="activeTab" /> <div class="mt-2 rounded-xl overflow-hidden border border-outline-gray-1"> <div v-if="activeTab === 'preview'"> - <div class=""> - <div + <div :class="[ 'bg-surface-white p-8 overflow-auto scrollbar flex gap-3 items-center', css, ]" > <slot /> - </div> </div> </div>src/components/TabButtons/TabButtons.vue (1)
110-134: Add stable data- hooks for styling/customization.*Per the coding guideline "Use stable styling hooks like
data-slot,data-state,data-size,data-variant,data-disabledfor component customization", the native<button>exposes none. Consumers that want to restyle (e.g., override checked background) have no stable hook and must rely on Tailwind class composition. Consider addingdata-slot="tab-button",data-state="checked|unchecked", anddata-disabledso consumers can target states without class surgery.♻️ Proposed addition
<button :type="button.type" :disabled="disabled" + data-slot="tab-button" + :data-state="checked ? 'checked' : 'unchecked'" + :data-disabled="disabled ? '' : undefined" :aria-label="..."As per coding guidelines: "Use stable styling hooks like
data-slot,data-state,data-size,data-variant,data-disabledfor component customization".docs/components/Docs/PropsTable.vue (1)
64-131: Required-prop marker (*) lacks a legend.The standalone
requiredcolumn is gone and required props are now indicated only by a subtle*next to the prop name (Line 99). Without a header annotation or footnote, readers may not know what the asterisk means. Consider documenting it in thePropcolumn header (e.g.Prop+ small hint “*required”) or addingtitle="required"/aria-label="required"on the span for accessibility.♿ Suggested tweak
- {{ x.name - }}<span v-if="x.required" class="text-ink-gray-5">*</span> + {{ x.name + }}<span + v-if="x.required" + class="text-ink-gray-5" + title="Required" + aria-label="required" + >*</span>docs/meta/Autocomplete.md (1)
7-40: Section label is ambiguous when only slots are present.This page previously had a
## Slotsheading; now the only section header is## API Referencedirectly followed by a<SlotsTable>. For components that expose only slots (or only emits), a reader landing on this page sees "API Reference" without any sub-heading clarifying what the table represents. Since the generator (scripts/propsgen.ts) emits this, consider having it also emit sub-headings (### Props,### Slots,### Emits) under## API Referencewhen the corresponding section is non-empty. Cosmetic; safe to defer.src/components/Dropdown/DropdownRenderContent.vue (1)
1-9: Consider tightening thecontentprop type and avoid recreating the inline component on each render.
content?: anyerases useful type information; since this is meant to hold VNodes captured from a slot, preferVNode | VNode[].:is="{ render: () => content }"constructs a brand-new component definition on every render, which means Vue treats it as a different component each time and will unmount/remount the subtree. For simple VNode forwarding, a functional component declared once (or rendering the VNodes directly) is cheaper and preserves state across updates.♻️ Suggested refactor
<script setup lang="ts"> -defineProps<{ - content?: any -}>() +import type { VNode } from 'vue' + +defineProps<{ + content?: VNode | VNode[] +}>() </script> <template> - <component :is="{ render: () => content }" /> + <component :is="renderer" /> </template> + +<script lang="ts"> +import { defineComponent, h } from 'vue' +const renderer = defineComponent({ + props: { content: { type: [Object, Array], default: () => [] } }, + setup: (p) => () => h('template', null, p.content as any), +}) +</script>Or, if the parent already has the VNodes, consider rendering them directly via
<component :is>on a functional component defined once at module scope rather than inline. Verify whether remount-on-update is actually causing any issue in the current usage before refactoring.src/components/Select/stories/DisabledLabel.vue (1)
1-47: Consider consolidating under aStates.vuestory.The coding guidelines for
src/components/**/stories/*.vueencourage focused, representative stories using common types likeSizes.vue,Variants.vue,States.vue,WithIcons.vue,Interactive.vue.DisabledLabel.vuedemonstrates a specific "disabled first option with empty-string value" pattern that could fit naturally inside a broaderStates.vuestory (disabled option, disabled select, etc.) to avoid proliferating narrowly-scoped story files.Non-blocking — if the intent is to explicitly showcase the empty-string-value gotcha called out in the tests, keeping it separate is also defensible.
As per coding guidelines: "Create focused and representative stories based on public APIs, including common types: Sizes.vue, Variants.vue, States.vue, WithIcons.vue, Interactive.vue".
src/components/Dropdown/stories/02_MenuPatterns.vue (1)
82-88: Implicitanyin switchonClickhandlers.The
onClick: (value) => ...handlers are declared in<script setup lang="ts">without a type forvalue, which will triggernoImplicitAnyunder strict TS. Annotate the parameter (e.g.,(value: boolean)) to stay aligned with the TypeScript-first authoring guideline.✏️ Suggested change
- onClick: (value) => console.log('Lock comments:', value), + onClick: (value: boolean) => console.log('Lock comments:', value), ... - onClick: (value) => console.log('Collaborative editing:', value), + onClick: (value: boolean) => console.log('Collaborative editing:', value),docs/components/Docs/EmitsTable.vue (1)
2-5: Reflect optional metadata in the table item type.The template already supports missing payload and description; the interface should allow that shape too.
Align the type with the rendered fallbacks
interface ItemProp { name: string - description: string - type: string + description?: string + type?: string }src/components/ItemList/ItemList.vue (1)
78-92: Potential:keycollisions across groups.
group.key ?? group.group ?? groupIndexwill fall back togroup.group(the human-readable label), so two groups with the same label produce duplicate Vue keys. Consider always composing withgroupIndexas a tiebreaker, or requiringkeyfor multi-group usage.Proposed fix
- :key="group.key ?? group.group ?? groupIndex" + :key="group.key ?? `${group.group ?? 'group'}-${groupIndex}`"src/components/Select/Select.cy.ts (1)
231-278: Form submission test may flake due to{esc}closing order.Typing
{esc}onbodyafter opening the menu should close it, but if focus has already moved, the escape may not reach the menu. Consider clicking outside or asserting the menu closed before clicking Submit, to avoid timing flakes.src/components/Dropdown/DropdownMenuItemContent.vue (1)
90-108:hasRenderableContentis duplicated inItemListRow.vue.The identical helper exists at
src/components/ItemList/ItemListRow.vueLines 43-61. Extract to a shared utility (e.g.src/utils/vnode.ts) to avoid drift.♻️ Proposed extraction
-function hasRenderableContent(nodes?: VNode[]): boolean { - if (!nodes?.length) return false - ... -} +import { hasRenderableContent } from '../../utils/vnode'src/components/ItemList/ItemListRow.vue (1)
77-97: Slot-function calls happen on every re-render.
$slots.prefix?.()and$slots.suffix?.()are invoked on every render both by thev-ifand inside the wrapper slot. For expensive slot content this doubles VNode creation. Consider caching via a computed that captures the slot result once, or accept the tradeoff (VNode creation is typically cheap).src/components/ItemList/types.ts (1)
5-15:[key: string]: anyescape hatch weakens the public type.The index signature in
ItemListItemerodes type safety for consumers since arbitrary keys of typeanyare allowed. If the intent is to let callers extend items with custom fields, prefer having consumers extend the interface themselves (the generic already supports this viaTItem extends ItemListItem). Consider removing it.src/components/Dropdown/types.ts (1)
75-96: Export the full public Dropdown type surface.
openadds a public model contract, and the component now has a substantial slot API, but this file only exports props/options. AddDropdownEmits,DropdownSlots,DropdownExposed, and namedDropdownPlacement/DropdownSidealiases so consumers and docs can import the same public contract.As per coding guidelines,
src/components/**/types.ts: “Create consistent TypeScript type exports for public components:ComponentNameProps,ComponentNameEmits,ComponentNameSlots,ComponentNameExposed, andComponentNameSize/ComponentNameVariant/ComponentNameThemeetc.”src/components/Select/types.ts (1)
45-59: Document and export the Select slot/emit contract.The new public slot props and emits should have JSDoc, and the file should expose aggregate
SelectSlots/SelectExposedtypes alongsideSelectPropsandSelectEmitsfor consistency.As per coding guidelines,
src/components/**/types.ts: “Create consistent TypeScript type exports for public components:ComponentNameProps,ComponentNameEmits,ComponentNameSlots,ComponentNameExposed…” and “Every public prop, emit, and slot should have a JSDoc description.”
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ec2f89ef-0797-4537-88d8-748feb1148e1
📒 Files selected for processing (97)
.github/scripts/docs-pages.sh.github/workflows/docs-preview-cleanup.yml.github/workflows/docs-preview.yml.github/workflows/site-deploy.ymlcomponents.d.tsdocs/.vitepress/config.tsdocs/.vitepress/plugins/componentTransformer.tsdocs/components/Docs/Demo.vuedocs/components/Docs/EmitsTable.vuedocs/components/Docs/PropsTable.vuedocs/components/Docs/SlotsTable.vuedocs/components/Layout.vuedocs/content/components.d.tsdocs/content/docs/components/dropdown.mddocs/content/docs/components/itemlist.mddocs/content/docs/components/select.mddocs/css/style.cssdocs/meta/Alert.mddocs/meta/Autocomplete.mddocs/meta/Avatar.mddocs/meta/Badge.mddocs/meta/Breadcrumbs.mddocs/meta/Button.mddocs/meta/Checkbox.mddocs/meta/CircularProgressBar.mddocs/meta/Combobox.mddocs/meta/DatePicker.mddocs/meta/Dialog.mddocs/meta/Divider.mddocs/meta/Dropdown.mddocs/meta/ErrorMessage.mddocs/meta/FormControl.mddocs/meta/ItemList.mddocs/meta/MonthPicker.mddocs/meta/MultiSelect.mddocs/meta/Password.mddocs/meta/Popover.mddocs/meta/Progress.mddocs/meta/Rating.mddocs/meta/Select.mddocs/meta/Sidebar.mddocs/meta/Slider.mddocs/meta/Switch.mddocs/meta/Tabs.mddocs/meta/TextEditor.mddocs/meta/TextInput.mddocs/meta/Textarea.mddocs/meta/TimePicker.mddocs/meta/Toast.mddocs/meta/Tooltip.mddocs/meta/Tree.mddocs/scripts/propsgen.tspackage.jsonsrc/components/Dropdown/Dropdown.cy.tssrc/components/Dropdown/Dropdown.vuesrc/components/Dropdown/DropdownMenuItemContent.vuesrc/components/Dropdown/DropdownMenuList.vuesrc/components/Dropdown/DropdownRenderContent.vuesrc/components/Dropdown/DropdownRenderContentAsChild.vuesrc/components/Dropdown/index.tssrc/components/Dropdown/stories/01_Basic.vuesrc/components/Dropdown/stories/02_MenuPatterns.vuesrc/components/Dropdown/stories/03_TriggerPatterns.vuesrc/components/Dropdown/stories/04_ContentCustomization.vuesrc/components/Dropdown/stories/04_UserMenu.vuesrc/components/Dropdown/stories/05_ContentCustomization.vuesrc/components/Dropdown/stories/CustomTrigger.vuesrc/components/Dropdown/stories/Examples.vuesrc/components/Dropdown/stories/Grouped.vuesrc/components/Dropdown/stories/Submenus.vuesrc/components/Dropdown/stories/Switches.vuesrc/components/Dropdown/types.tssrc/components/Dropdown/utils.tssrc/components/ItemList/ItemList.cy.tssrc/components/ItemList/ItemList.vuesrc/components/ItemList/ItemListRow.cy.tssrc/components/ItemList/ItemListRow.vuesrc/components/ItemList/index.tssrc/components/ItemList/stories/AdvancedSlots.vuesrc/components/ItemList/stories/Basic.vuesrc/components/ItemList/stories/CustomSlots.vuesrc/components/ItemList/stories/EmptyAndFooter.vuesrc/components/ItemList/stories/RowStates.vuesrc/components/ItemList/types.tssrc/components/Select/Select.cy.tssrc/components/Select/Select.vuesrc/components/Select/index.tssrc/components/Select/stories/DisabledLabel.vuesrc/components/Select/stories/Example.vuesrc/components/Select/stories/OptionSlot.vuesrc/components/Select/stories/TriggerSlots.vuesrc/components/Select/types.tssrc/components/TabButtons/TabButtons.cy.tssrc/components/TabButtons/TabButtons.vuesrc/index.tstsconfig.jsonv1-release/08-selection-and-menu-api-spec.md
💤 Files with no reviewable changes (5)
- src/components/Dropdown/stories/Grouped.vue
- src/components/Dropdown/stories/Submenus.vue
- src/components/Dropdown/stories/CustomTrigger.vue
- src/components/Dropdown/stories/Examples.vue
- src/components/Dropdown/stories/Switches.vue
| prepare_pages_worktree() { | ||
| rm -rf "$PAGES_DIR" | ||
| git fetch origin "$PAGES_BRANCH:refs/remotes/origin/$PAGES_BRANCH" || true | ||
|
|
||
| if git show-ref --verify --quiet "refs/remotes/origin/$PAGES_BRANCH"; then | ||
| git worktree add -B "$PAGES_BRANCH" "$PAGES_DIR" "origin/$PAGES_BRANCH" | ||
| return | ||
| fi | ||
|
|
||
| git worktree add -B "$PAGES_BRANCH" "$PAGES_DIR" HEAD | ||
| find "$PAGES_DIR" -mindepth 1 -maxdepth 1 ! -name '.git' -exec rm -rf {} + | ||
| } |
There was a problem hiding this comment.
Silent fetch failure can lead to a HEAD-based branch recreation.
git fetch ... || true swallows network errors. If the fetch fails but origin/gh-pages has not been previously fetched, show-ref misses it and the script falls through to creating gh-pages from HEAD and wiping files (line 19). A subsequent push would likely be rejected by the remote (good), but the behavior is surprising. Consider failing fast on fetch errors unless the remote branch truly does not exist (use git ls-remote --exit-code origin "$PAGES_BRANCH" to distinguish).
| permissions: | ||
| contents: write | ||
| pull-requests: write | ||
|
|
||
| concurrency: | ||
| group: docs-pages | ||
| cancel-in-progress: false | ||
|
|
||
| jobs: | ||
| preview: | ||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v5 | ||
| with: | ||
| fetch-depth: 0 | ||
|
|
||
| - name: Setup Node | ||
| uses: actions/setup-node@v6 | ||
| with: | ||
| node-version: 24 | ||
| cache: yarn | ||
|
|
||
| - name: Install dependencies | ||
| run: yarn install --frozen-lockfile | ||
|
|
||
| - name: Build preview | ||
| run: VITEPRESS_BASE=/pr-preview/pr-${{ github.event.pull_request.number }}/ yarn docs:build | ||
|
|
||
| - name: Publish preview | ||
| if: ${{ github.event.pull_request.head.repo.full_name == github.repository }} | ||
| run: bash .github/scripts/docs-pages.sh deploy-preview docs/.vitepress/dist ${{ github.event.pull_request.number }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the actual workflow file
find . -type f -name "docs-preview.yml" | head -20Repository: frappe/frappe-ui
Length of output: 96
🏁 Script executed:
# Let's also check the repository structure
ls -la .github/workflows/ 2>/dev/null | head -20Repository: frappe/frappe-ui
Length of output: 565
🌐 Web query:
actions/checkout v5 default behavior persist-credentials GITHUB_TOKEN
💡 Result:
In actions/checkout v5, the default behavior for persist-credentials is true. When using the default token: ${{ github.token }} (GITHUB_TOKEN), this persists the token in the local git config (stored in a secure location under $RUNNER_TEMP in later versions, but still accessible), enabling subsequent scripts in the job to run authenticated git commands like git push without explicitly passing the token. The token is automatically removed during post-job cleanup. To opt-out and prevent persistence, explicitly set persist-credentials: false. Note: GITHUB_TOKEN is available to all steps in a job regardless, but persistence provides implicit git credential access. For security, limit GITHUB_TOKEN permissions (e.g., contents: read) and consider persist-credentials: false if not needed.
Citations:
- 1: https://github.com/actions/checkout/blob/main/README.md
- 2: https://github.com/actions/checkout?tab=readme-ov-file
- 3: https://github.com/actions/checkout
🌐 Web query:
GitHub Actions GITHUB_TOKEN access during yarn install build scripts security risk
💡 Result:
Yes, there is a significant security risk with GITHUB_TOKEN access during yarn install and build scripts in GitHub Actions. The GITHUB_TOKEN is automatically available as an environment variable in every job, accessible to any code executed, including yarn postinstall/preinstall scripts and build scripts like yarn build. Malicious or compromised packages can exfiltrate the token via techniques like curl to an attacker server, as shown in official GitHub docs examples of token theft (e.g., curl http://example.com?token=$GITHUB_TOKEN). Real-world incidents confirm this: malicious npm packages like @acitons/artifact (206k+ downloads) specifically target GitHub Actions to steal GITHUB_TOKEN via postinstall hooks; Orval CI workflow vulnerable to prepack scripts exfiltrating tokens; supply chain attacks like Trivy breach exploiting workflows. Even trusted dependencies pose risks if compromised (e.g., tj-actions/changed-files incident). GITHUB_TOKEN has repo-scoped write permissions by default (contents: write, etc.), allowing stolen tokens to push malicious code, create releases, or modify repo before expiration (short-lived, job-bound). Mitigations: - Set workflow permissions to least privilege: permissions: { contents: read } for build-only jobs. - Use permissions: { contents: none } if possible. - Repository/Org setting: restrict default workflow permissions to read-only. - Avoid running untrusted yarn scripts; use --ignore-scripts (may break builds). - Pin dependencies, audit with tools like Socket/Trivy. - For npm auth in yarn, configure .yarnrc/npmrc safely without exposing token further. Official docs emphasize: actions access GITHUB_TOKEN via github.token even if not passed, so limit permissions always.
Citations:
- 1: https://help.github.com/en/actions/concepts/security/compromised-runners
- 2: https://docs.github.com/en/actions/concepts/security/compromised-runners
- 3: https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs/
- 4: https://docs.github.com/en/actions/concepts/security/github_token
- 5: https://hackyourmom.com/en/novyny/shkidlyvyj-npm-paket-u-github-acitons-artifact-polyuvav-na-tokeny-github-repozytoriyiv/
- 6: https://ift.tt/X5Wi8uf
- 7: GHSA-c2x7-22rq-26fr
- 8: https://orca.security/resources/blog/github-actions-security-risks/
- 9: Restrict GITHUB_TOKEN permissions on GitHub Actions Kong/kong#14778
- 10: https://securityboulevard.com/2026/04/github-actions-supply-chain-attack-trivy-breach-workflow/
🏁 Script executed:
# Let's examine the actual workflow file in detail
cat -n .github/workflows/docs-preview.ymlRepository: frappe/frappe-ui
Length of output: 3280
Don't expose write token to PR-controlled build steps.
This job grants contents: write and pull-requests: write at the workflow level, making them available during yarn install (line 32) and yarn docs:build (line 35), before the same-repo guard at line 38. For same-repo PRs, malicious or compromised dependencies can access GITHUB_TOKEN via postinstall/preinstall hooks and build scripts to exfiltrate credentials or push malicious code. This is a known attack vector documented by GitHub and confirmed in real incidents.
Separate into a read-only build job and a restricted publish job:
- Build job: run with
permissions: { contents: read }, setpersist-credentials: false, and upload artifact. - Publish job: run only after build succeeds, with same-repo guard,
permissions: { contents: write, pull-requests: write }, and download artifact for deploy.
See GitHub's job-level permissions documentation: https://docs.github.com/en/actions/configuring-and-managing-workflows/authenticating-with-the-github_token
Safer workflow shape
-permissions:
- contents: write
- pull-requests: write
+permissions:
+ contents: read
jobs:
- preview:
+ build-preview:
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v5
with:
fetch-depth: 0
+ persist-credentials: false
...
- name: Build preview
run: VITEPRESS_BASE=/pr-preview/pr-${{ github.event.pull_request.number }}/ yarn docs:build
+ - name: Upload preview artifact
+ uses: actions/upload-artifact@v4
+ with:
+ name: docs-preview
+ path: docs/.vitepress/dist
+
+ publish-preview:
+ if: ${{ github.event.pull_request.head.repo.full_name == github.repository }}
+ needs: build-preview
+ runs-on: ubuntu-latest
+ permissions:
+ contents: write
+ pull-requests: write
+ steps:
+ - name: Checkout trusted deploy scripts
+ uses: actions/checkout@v5
+ - name: Download preview artifact
+ uses: actions/download-artifact@v4
+ with:
+ name: docs-preview
+ path: docs/.vitepress/dist
+ - name: Publish preview
+ run: bash .github/scripts/docs-pages.sh deploy-preview docs/.vitepress/dist ${{ github.event.pull_request.number }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| permissions: | |
| contents: write | |
| pull-requests: write | |
| concurrency: | |
| group: docs-pages | |
| cancel-in-progress: false | |
| jobs: | |
| preview: | |
| runs-on: ubuntu-latest | |
| steps: | |
| - name: Checkout | |
| uses: actions/checkout@v5 | |
| with: | |
| fetch-depth: 0 | |
| - name: Setup Node | |
| uses: actions/setup-node@v6 | |
| with: | |
| node-version: 24 | |
| cache: yarn | |
| - name: Install dependencies | |
| run: yarn install --frozen-lockfile | |
| - name: Build preview | |
| run: VITEPRESS_BASE=/pr-preview/pr-${{ github.event.pull_request.number }}/ yarn docs:build | |
| - name: Publish preview | |
| if: ${{ github.event.pull_request.head.repo.full_name == github.repository }} | |
| run: bash .github/scripts/docs-pages.sh deploy-preview docs/.vitepress/dist ${{ github.event.pull_request.number }} | |
| permissions: | |
| contents: read | |
| concurrency: | |
| group: docs-pages | |
| cancel-in-progress: false | |
| jobs: | |
| build-preview: | |
| runs-on: ubuntu-latest | |
| steps: | |
| - name: Checkout | |
| uses: actions/checkout@v5 | |
| with: | |
| fetch-depth: 0 | |
| persist-credentials: false | |
| - name: Setup Node | |
| uses: actions/setup-node@v6 | |
| with: | |
| node-version: 24 | |
| cache: yarn | |
| - name: Install dependencies | |
| run: yarn install --frozen-lockfile | |
| - name: Build preview | |
| run: VITEPRESS_BASE=/pr-preview/pr-${{ github.event.pull_request.number }}/ yarn docs:build | |
| - name: Upload preview artifact | |
| uses: actions/upload-artifact@v4 | |
| with: | |
| name: docs-preview | |
| path: docs/.vitepress/dist | |
| publish-preview: | |
| if: ${{ github.event.pull_request.head.repo.full_name == github.repository }} | |
| needs: build-preview | |
| runs-on: ubuntu-latest | |
| permissions: | |
| contents: write | |
| pull-requests: write | |
| steps: | |
| - name: Checkout trusted deploy scripts | |
| uses: actions/checkout@v5 | |
| - name: Download preview artifact | |
| uses: actions/download-artifact@v4 | |
| with: | |
| name: docs-preview | |
| path: docs/.vitepress/dist | |
| - name: Publish preview | |
| run: bash .github/scripts/docs-pages.sh deploy-preview docs/.vitepress/dist ${{ github.event.pull_request.number }} |
| 01_Basic: typeof import('./src/components/Dropdown/stories/01_Basic.vue')['default'] | ||
| 02_MenuPatterns: typeof import('./src/components/Dropdown/stories/02_MenuPatterns.vue')['default'] | ||
| 03_TriggerPatterns: typeof import('./src/components/Dropdown/stories/03_TriggerPatterns.vue')['default'] | ||
| 04_ContentCustomization: typeof import('./src/components/Dropdown/stories/04_ContentCustomization.vue')['default'] | ||
| 04_UserMenu: typeof import('./src/components/Dropdown/stories/04_UserMenu.vue')['default'] | ||
| 05_ContentCustomization: typeof import('./src/components/Dropdown/stories/05_ContentCustomization.vue')['default'] |
There was a problem hiding this comment.
Quote numeric-leading global component names.
These declarations are invalid TypeScript identifiers, which matches the Biome parse failure. Quote them as string-literal property names, or rename/configure the generated story component names so the declaration file remains parseable.
🐛 Proposed fix
- 01_Basic: typeof import('./src/components/Dropdown/stories/01_Basic.vue')['default']
- 02_MenuPatterns: typeof import('./src/components/Dropdown/stories/02_MenuPatterns.vue')['default']
- 03_TriggerPatterns: typeof import('./src/components/Dropdown/stories/03_TriggerPatterns.vue')['default']
- 04_ContentCustomization: typeof import('./src/components/Dropdown/stories/04_ContentCustomization.vue')['default']
- 04_UserMenu: typeof import('./src/components/Dropdown/stories/04_UserMenu.vue')['default']
- 05_ContentCustomization: typeof import('./src/components/Dropdown/stories/05_ContentCustomization.vue')['default']
+ '01_Basic': typeof import('./src/components/Dropdown/stories/01_Basic.vue')['default']
+ '02_MenuPatterns': typeof import('./src/components/Dropdown/stories/02_MenuPatterns.vue')['default']
+ '03_TriggerPatterns': typeof import('./src/components/Dropdown/stories/03_TriggerPatterns.vue')['default']
+ '04_ContentCustomization': typeof import('./src/components/Dropdown/stories/04_ContentCustomization.vue')['default']
+ '04_UserMenu': typeof import('./src/components/Dropdown/stories/04_UserMenu.vue')['default']
+ '05_ContentCustomization': typeof import('./src/components/Dropdown/stories/05_ContentCustomization.vue')['default']📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 01_Basic: typeof import('./src/components/Dropdown/stories/01_Basic.vue')['default'] | |
| 02_MenuPatterns: typeof import('./src/components/Dropdown/stories/02_MenuPatterns.vue')['default'] | |
| 03_TriggerPatterns: typeof import('./src/components/Dropdown/stories/03_TriggerPatterns.vue')['default'] | |
| 04_ContentCustomization: typeof import('./src/components/Dropdown/stories/04_ContentCustomization.vue')['default'] | |
| 04_UserMenu: typeof import('./src/components/Dropdown/stories/04_UserMenu.vue')['default'] | |
| 05_ContentCustomization: typeof import('./src/components/Dropdown/stories/05_ContentCustomization.vue')['default'] | |
| '01_Basic': typeof import('./src/components/Dropdown/stories/01_Basic.vue')['default'] | |
| '02_MenuPatterns': typeof import('./src/components/Dropdown/stories/02_MenuPatterns.vue')['default'] | |
| '03_TriggerPatterns': typeof import('./src/components/Dropdown/stories/03_TriggerPatterns.vue')['default'] | |
| '04_ContentCustomization': typeof import('./src/components/Dropdown/stories/04_ContentCustomization.vue')['default'] | |
| '04_UserMenu': typeof import('./src/components/Dropdown/stories/04_UserMenu.vue')['default'] | |
| '05_ContentCustomization': typeof import('./src/components/Dropdown/stories/05_ContentCustomization.vue')['default'] |
🧰 Tools
🪛 Biome (2.4.11)
[error] 11-11: Expected a property, or a signature but instead found '01_Basic'.
(parse)
[error] 11-11: numeric separator can not be used after leading 0
(parse)
[error] 11-11: numeric separators are only allowed between two digits
(parse)
[error] 11-11: numbers cannot be followed by identifiers directly after
(parse)
[error] 12-12: Expected a statement but instead found '02_MenuPatterns: typeof'.
(parse)
[error] 12-12: numeric separator can not be used after leading 0
(parse)
[error] 12-12: numeric separators are only allowed between two digits
(parse)
[error] 12-12: numbers cannot be followed by identifiers directly after
(parse)
[error] 13-13: Expected a statement but instead found '03_TriggerPatterns: typeof'.
(parse)
[error] 13-13: numeric separator can not be used after leading 0
(parse)
[error] 13-13: numeric separators are only allowed between two digits
(parse)
[error] 13-13: numbers cannot be followed by identifiers directly after
(parse)
[error] 14-14: Expected a statement but instead found '04_ContentCustomization: typeof'.
(parse)
[error] 14-14: numeric separator can not be used after leading 0
(parse)
[error] 14-14: numeric separators are only allowed between two digits
(parse)
[error] 14-14: numbers cannot be followed by identifiers directly after
(parse)
[error] 15-15: Expected a statement but instead found '04_UserMenu: typeof'.
(parse)
[error] 15-15: numeric separator can not be used after leading 0
(parse)
[error] 15-15: numeric separators are only allowed between two digits
(parse)
[error] 15-15: numbers cannot be followed by identifiers directly after
(parse)
[error] 16-16: Expected a statement but instead found '05_ContentCustomization: typeof'.
(parse)
[error] 16-16: numeric separator can not be used after leading 0
(parse)
[error] 16-16: numeric separators are only allowed between two digits
(parse)
[error] 16-16: numbers cannot be followed by identifiers directly after
(parse)
| </colgroup> | ||
| <template> | ||
| <div class="not-prose mt-2"> | ||
| <details class="group border-outline-gray-2 pb-3"> |
There was a problem hiding this comment.
Orphan border color class—no border will render.
border-outline-gray-2 is a Tailwind border-color utility; without a corresponding width class (border, border-b, etc.) no border is actually painted. Either add a width class or drop border-outline-gray-2 to avoid the dead style. If the intent was the horizontal rule that used to separate this section, you likely want border-b border-outline-gray-2.
🎨 Suggested fix
- <details class="group border-outline-gray-2 pb-3">
+ <details class="group border-b border-outline-gray-2 pb-3">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <details class="group border-outline-gray-2 pb-3"> | |
| <details class="group border-b border-outline-gray-2 pb-3"> |
| <td class="px-2 py-2 align-top"> | ||
| <div | ||
| v-if="x.default?.includes('{')" | ||
| class="whitespace-pre-wrap break-words font-mono text-xs leading-6 text-ink-gray-6" | ||
| > | ||
| {{ x.default }} | ||
| </div> | ||
| <div v-else class="font-mono text-xs leading-6 text-ink-gray-6"> | ||
| {{ x.default || '—' }} | ||
| </div> | ||
| </td> |
There was a problem hiding this comment.
Brittle heuristic for detecting multi-line default values.
x.default?.includes('{') will also match ordinary string defaults that happen to contain a { (e.g. template literals, i18n placeholders like "Hello {name}", JSON-ish strings), causing them to render with whitespace-pre-wrap break-words unnecessarily. Consider a stricter check (startsWith('{') and endsWith('}') after trimming, or length > N), or always use whitespace-pre-wrap and rely on break-words for layout.
♻️ Suggested fix
- <div
- v-if="x.default?.includes('{')"
- class="whitespace-pre-wrap break-words font-mono text-xs leading-6 text-ink-gray-6"
- >
- {{ x.default }}
- </div>
- <div v-else class="font-mono text-xs leading-6 text-ink-gray-6">
- {{ x.default || '—' }}
- </div>
+ <div
+ class="whitespace-pre-wrap break-words font-mono text-xs leading-6 text-ink-gray-6"
+ >
+ {{ x.default || '—' }}
+ </div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <td class="px-2 py-2 align-top"> | |
| <div | |
| v-if="x.default?.includes('{')" | |
| class="whitespace-pre-wrap break-words font-mono text-xs leading-6 text-ink-gray-6" | |
| > | |
| {{ x.default }} | |
| </div> | |
| <div v-else class="font-mono text-xs leading-6 text-ink-gray-6"> | |
| {{ x.default || '—' }} | |
| </div> | |
| </td> | |
| <td class="px-2 py-2 align-top"> | |
| <div | |
| class="whitespace-pre-wrap break-words font-mono text-xs leading-6 text-ink-gray-6" | |
| > | |
| {{ x.default || '—' }} | |
| </div> | |
| </td> |
|
|
||
| const formAttrKeys = ['name', 'required', 'autocomplete'] as const | ||
| const keyboardOpenKeys = new Set(['Enter', ' ', 'ArrowDown', 'ArrowUp']) | ||
| const emptyValuePrefix = '__frappe_ui_select_empty__' |
There was a problem hiding this comment.
Avoid converting real option values that share the empty sentinel prefix.
A legitimate string value like __frappe_ui_select_empty__foo will round-trip through toExternalValue() as '', corrupting the selected value. Use an unforgeable internal sentinel, or map back only when the selected internal value matches a generated empty-option entry.
🐛 Proposed direction
-const emptyValuePrefix = '__frappe_ui_select_empty__'
+const emptyValueMarker = Symbol('frappe-ui-select-empty-value')
+
+function createEmptyValue(index: number) {
+ return { [emptyValueMarker]: index }
+}
const internalOptions = computed(() => {
return selectOptions.value.map((option, index) => ({
option,
internalValue:
- option.value === '' ? `${emptyValuePrefix}${index}` : option.value,
+ option.value === '' ? createEmptyValue(index) : option.value,
}))
})
function toExternalValue(value: SelectOptionValue | undefined) {
- if (typeof value === 'string' && value.startsWith(emptyValuePrefix)) {
- return ''
- }
-
- return value
+ return (
+ internalOptions.value.find((option) => option.internalValue === value)
+ ?.option.value ?? value
+ )
}Also applies to: 160-180
| <SelectItem | ||
| v-for="internalOption in internalOptions" | ||
| :key="String(internalOption.internalValue)" | ||
| :disabled="internalOption.option.disabled" | ||
| :value="internalOption.internalValue" |
There was a problem hiding this comment.
Use collision-safe keys for supported option value types.
String(internalOption.internalValue) collapses distinct values such as 1 and '1', and every object value becomes [object Object], causing duplicate Vue keys for valid SelectOptionValues.
🐛 Proposed fix
- <SelectItem
- v-for="internalOption in internalOptions"
- :key="String(internalOption.internalValue)"
+ <SelectItem
+ v-for="(internalOption, index) in internalOptions"
+ :key="`${index}:${typeof internalOption.internalValue}:${String(internalOption.internalValue)}`"| <script setup lang="ts"> | ||
| import { computed, type Component } from 'vue' | ||
| import { RadioGroupRoot, RadioGroupItem } from 'reka-ui' | ||
| import FeatherIcon from '../FeatherIcon.vue' | ||
|
|
||
| defineOptions({ | ||
| name: 'TabButtons', | ||
| inheritAttrs: false, | ||
| }) | ||
|
|
||
| type TabButtonValue = string | number | boolean | ||
|
|
||
| type TabButtonIcon = string | Component | ||
|
|
||
| type NativeButtonClass = string | string[] | Record<string, boolean> | ||
|
|
||
| interface TabButton { | ||
| label?: string | number | ||
| value?: TabButtonValue | ||
| icon?: TabButtonIcon | ||
| iconLeft?: TabButtonIcon | ||
| iconRight?: TabButtonIcon | ||
| hideLabel?: boolean | ||
| disabled?: boolean | ||
| active?: boolean | ||
| type?: 'button' | 'submit' | 'reset' | ||
| class?: NativeButtonClass | ||
| onClick?: (event: MouseEvent) => void | ||
| } | ||
|
|
||
| const props = defineProps<{ | ||
| buttons: TabButton[] | ||
| modelValue?: TabButtonValue | ||
| }>() | ||
|
|
||
| const emit = defineEmits<{ | ||
| 'update:modelValue': [value: TabButtonValue | undefined] | ||
| }>() | ||
|
|
||
| const resolvedButtons = computed(() => { | ||
| return props.buttons.map((button, index) => { | ||
| const { | ||
| value, | ||
| label, | ||
| icon, | ||
| iconLeft, | ||
| iconRight, | ||
| hideLabel = false, | ||
| disabled = false, | ||
| active = false, | ||
| type = 'button', | ||
| class: customClass, | ||
| onClick, | ||
| } = button | ||
|
|
||
| return { | ||
| key: `tab-button-${index}`, | ||
| label, | ||
| hideLabel, | ||
| disabled, | ||
| active, | ||
| type, | ||
| customClass, | ||
| onClick, | ||
| modelValue: value ?? label ?? index, | ||
| leadingIcon: iconLeft ?? icon, | ||
| trailingIcon: iconRight, | ||
| } | ||
| }) | ||
| }) | ||
|
|
||
| const selectedButtonKey = computed({ | ||
| get: () => { | ||
| const selectedButton = resolvedButtons.value.find((button) => | ||
| Object.is(button.modelValue, props.modelValue), | ||
| ) | ||
|
|
||
| if (selectedButton) { | ||
| return selectedButton.key | ||
| } | ||
|
|
||
| return resolvedButtons.value.find((button) => button.active)?.key | ||
| }, | ||
| set: (nextKey) => { | ||
| const selectedButton = resolvedButtons.value.find( | ||
| (button) => button.key === nextKey, | ||
| ) | ||
| emit('update:modelValue', selectedButton?.modelValue) | ||
| }, | ||
| }) | ||
|
|
||
| function hasLabel(label: TabButton['label']) { | ||
| return label !== undefined && label !== null && label !== '' | ||
| } | ||
| </script> | ||
|
|
||
| <template> | ||
| <RadioGroup v-model="value"> | ||
| <RadioGroupRoot v-model="selectedButtonKey" v-bind="$attrs"> | ||
| <div | ||
| class="flex space-x-0.5 rounded-md bg-surface-gray-2 h-7 items-center px-[1px] text-sm" | ||
| class="inline-flex min-h-7 items-center gap-0.5 rounded-md bg-surface-gray-2 p-px ring-1 ring-inset ring-outline-gray-1" | ||
| > | ||
| <RadioGroupOption | ||
| as="div" | ||
| v-for="button in buttons" | ||
| :key="button.label" | ||
| <RadioGroupItem | ||
| v-for="button in resolvedButtons" | ||
| :key="button.key" | ||
| v-slot="{ checked, disabled }" | ||
| as="template" | ||
| :disabled="button.disabled" | ||
| :value="button.value ?? button.label" | ||
| v-slot="{ active, checked }" | ||
| :value="button.key" | ||
| > | ||
| <Button | ||
| @click="button.onClick" | ||
| v-bind="button" | ||
| class="!h-6.5" | ||
| <button | ||
| :type="button.type" | ||
| :disabled="disabled" | ||
| :aria-label=" | ||
| button.hideLabel && hasLabel(button.label) | ||
| ? String(button.label) | ||
| : undefined | ||
| " | ||
| :title=" | ||
| button.hideLabel && hasLabel(button.label) | ||
| ? String(button.label) | ||
| : undefined | ||
| " | ||
| class="inline-flex h-7 shrink-0 items-center justify-center gap-2 rounded-[9px] border border-transparent text-base transition-[transform,background-color,color,box-shadow,border-color] duration-150 ease-out motion-safe:active:scale-[0.98] motion-reduce:transform-none motion-reduce:transition-none" | ||
| :class="[ | ||
| active ? 'ring-outline-gray-2 focus-visible:ring' : '', | ||
| checked && '!bg-surface-white', | ||
| button.disabled | ||
| ? '' | ||
| : checked | ||
| ? ' text-ink-gray-8 shadow' | ||
| : '!text-ink-gray-5', | ||
| button.hideLabel ? 'w-7 px-0' : 'px-2.5', | ||
| checked | ||
| ? 'border-outline-gray-1 bg-surface-white text-ink-gray-8 shadow-sm' | ||
| : disabled | ||
| ? 'bg-transparent text-ink-gray-4' | ||
| : 'bg-transparent text-ink-gray-5 hover:bg-surface-gray-3/80 hover:text-ink-gray-7', | ||
| button.customClass, | ||
| ]" | ||
| @click="button.onClick?.($event)" | ||
| > | ||
| <RadioGroupLabel | ||
| as="span" | ||
| class="flex h-4 items-center" | ||
| v-show="button.label && !button.hideLabel" | ||
| >{{ button.label }}</RadioGroupLabel | ||
| <FeatherIcon | ||
| v-if="button.leadingIcon && typeof button.leadingIcon === 'string'" | ||
| :name="button.leadingIcon" | ||
| class="h-4 w-4 shrink-0" | ||
| aria-hidden="true" | ||
| /> | ||
| <component | ||
| :is="button.leadingIcon" | ||
| v-else-if="button.leadingIcon" | ||
| class="h-4 w-4 shrink-0" | ||
| aria-hidden="true" | ||
| /> | ||
|
|
||
| <span | ||
| v-if="hasLabel(button.label)" | ||
| class="flex min-w-0 items-center truncate" | ||
| :class="button.hideLabel ? 'sr-only' : undefined" | ||
| > | ||
| </Button> | ||
| </RadioGroupOption> | ||
| {{ button.label }} | ||
| </span> | ||
|
|
||
| <FeatherIcon | ||
| v-if=" | ||
| button.trailingIcon && typeof button.trailingIcon === 'string' | ||
| " | ||
| :name="button.trailingIcon" | ||
| class="h-4 w-4 shrink-0" | ||
| aria-hidden="true" | ||
| /> | ||
| <component | ||
| :is="button.trailingIcon" | ||
| v-else-if="button.trailingIcon" | ||
| class="h-4 w-4 shrink-0" | ||
| aria-hidden="true" | ||
| /> | ||
| </button> | ||
| </RadioGroupItem> | ||
| </div> | ||
| </RadioGroup> | ||
| </RadioGroupRoot> | ||
| </template> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all usages of TabButtons across the repo and inspect `:buttons` shapes
rg -nP -C3 --type=vue --type=ts '\bTabButtons\b'
# Also check docs/stories for field usage
fd -e vue -e ts | xargs rg -nP -C2 '\b(iconLeft|iconRight|hideLabel|slug|theme|variant)\b' -g '*TabButton*' 2>/dev/nullRepository: frappe/frappe-ui
Length of output: 50418
🏁 Script executed:
# Find TabButtons usage patterns - exclude noise
rg -n 'TabButtons' --type vue --type ts -g '*TabButton*' src/Repository: frappe/frappe-ui
Length of output: 87
🏁 Script executed:
# Check TabButtons stories/tests for field usage patterns
find src/components/TabButtons -type f \( -name "*.vue" -o -name "*.ts" \)Repository: frappe/frappe-ui
Length of output: 225
🏁 Script executed:
# Look for any old TabButtons references in git history or comments
git log --all --oneline -- src/components/TabButtons/TabButtons.vue | head -5Repository: frappe/frappe-ui
Length of output: 331
🏁 Script executed:
# Search for any usages of old field names in context of TabButtons
rg -n '\b(slug|hidden|theme|variant)\b' src/components/TabButtons/ --type vue --type tsRepository: frappe/frappe-ui
Length of output: 87
🏁 Script executed:
cat -n src/components/TabButtons/TabButtons.cy.tsRepository: frappe/frappe-ui
Length of output: 2047
🏁 Script executed:
cat -n src/components/TabButtons/TabButtons.story.vueRepository: frappe/frappe-ui
Length of output: 717
🏁 Script executed:
# Check what changed in recent commits
git show 0ed209f:src/components/TabButtons/TabButtons.vue | head -80Repository: frappe/frappe-ui
Length of output: 1818
🏁 Script executed:
# Check the Button component to see what props it accepted
git show 0ed209f:src/components/Button/Button.vue | head -150Repository: frappe/frappe-ui
Length of output: 5697
🏁 Script executed:
# Search the entire codebase for any use of TabButtons with variant or theme
rg -n 'TabButtons' -A 5 -B 5 src/ | grep -i 'variant\|theme'Repository: frappe/frappe-ui
Length of output: 410
🏁 Script executed:
# Check current Button component to see what it exposes
head -150 src/components/Button/Button.vueRepository: frappe/frappe-ui
Length of output: 5640
🏁 Script executed:
# Check if there are any actual usages of TabButtons with variant or theme in the codebase
rg -n 'buttons.*variant|buttons.*theme' src/ --type ts --type vue -iRepository: frappe/frappe-ui
Length of output: 87
🏁 Script executed:
# Get the Button types to confirm what was available
head -80 src/components/Button/types.tsRepository: frappe/frappe-ui
Length of output: 1334
This is a breaking change that removes support for Button component properties.
The new TabButtons implementation changes the button shape from accepting any Button component props (via v-bind="button") to an explicit TabButton interface that omits theme, variant, size, and other Button properties. Any existing code passing these properties will silently ignore them:
variant('solid', 'subtle', 'outline', 'ghost') - no longer supportedtheme('gray', 'blue', 'green', 'red') - no longer supportedsize,loading,tooltip,route,link- no longer supportedtypedefault remains 'button' (safe, no form-submit regression)
The new styling approach uses hardcoded Tailwind utilities rather than the Button component's theme/variant system. While the class prop allows per-button customization, it lacks stable styling hooks (data-variant, data-state, etc.) for predictable CSS overrides.
Existing usages should be audited to confirm they don't rely on the old properties, or this change should be treated as a planned breaking change with a migration guide.
| const selectedButtonKey = computed({ | ||
| get: () => { | ||
| const selectedButton = resolvedButtons.value.find((button) => | ||
| Object.is(button.modelValue, props.modelValue), | ||
| ) | ||
|
|
||
| if (selectedButton) { | ||
| return selectedButton.key | ||
| } | ||
|
|
||
| return resolvedButtons.value.find((button) => button.active)?.key | ||
| }, | ||
| set: (nextKey) => { | ||
| const selectedButton = resolvedButtons.value.find( | ||
| (button) => button.key === nextKey, | ||
| ) | ||
| emit('update:modelValue', selectedButton?.modelValue) | ||
| }, | ||
| }) |
There was a problem hiding this comment.
active fallback can desync modelValue with the visually checked tab.
When props.modelValue doesn't match any resolvedButtons[].modelValue, the getter returns the key of the first active: true button, so RadioGroupRoot renders it as checked — but no update:modelValue is emitted. The parent's model is now out of sync with the UI. Worse, if the user clicks that already-checked button, reka-ui won't emit a change (same key) and the parent will never converge.
Consider either (a) emitting an initial update:modelValue in an effect when falling back, or (b) keeping the getter deterministic (return only matched keys, i.e. no active fallback) and letting consumers pass the correct initial modelValue. Option (a) is closer to the prior behavior:
🔧 Suggested approach
+import { watch } from 'vue'
+
+// Sync parent with the `active` fallback on mount / when buttons change
+watch(
+ () => resolvedButtons.value,
+ (buttons) => {
+ const matched = buttons.find((b) => Object.is(b.modelValue, props.modelValue))
+ if (matched) return
+ const fallback = buttons.find((b) => b.active)
+ if (fallback && !Object.is(fallback.modelValue, props.modelValue)) {
+ emit('update:modelValue', fallback.modelValue)
+ }
+ },
+ { immediate: true },
+)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const selectedButtonKey = computed({ | |
| get: () => { | |
| const selectedButton = resolvedButtons.value.find((button) => | |
| Object.is(button.modelValue, props.modelValue), | |
| ) | |
| if (selectedButton) { | |
| return selectedButton.key | |
| } | |
| return resolvedButtons.value.find((button) => button.active)?.key | |
| }, | |
| set: (nextKey) => { | |
| const selectedButton = resolvedButtons.value.find( | |
| (button) => button.key === nextKey, | |
| ) | |
| emit('update:modelValue', selectedButton?.modelValue) | |
| }, | |
| }) | |
| import { watch } from 'vue' | |
| // ... other component code ... | |
| // Sync parent with the `active` fallback on mount / when buttons change | |
| watch( | |
| () => resolvedButtons.value, | |
| (buttons) => { | |
| const matched = buttons.find((b) => Object.is(b.modelValue, props.modelValue)) | |
| if (matched) return | |
| const fallback = buttons.find((b) => b.active) | |
| if (fallback && !Object.is(fallback.modelValue, props.modelValue)) { | |
| emit('update:modelValue', fallback.modelValue) | |
| } | |
| }, | |
| { immediate: true }, | |
| ) | |
| const selectedButtonKey = computed({ | |
| get: () => { | |
| const selectedButton = resolvedButtons.value.find((button) => | |
| Object.is(button.modelValue, props.modelValue), | |
| ) | |
| if (selectedButton) { | |
| return selectedButton.key | |
| } | |
| return resolvedButtons.value.find((button) => button.active)?.key | |
| }, | |
| set: (nextKey) => { | |
| const selectedButton = resolvedButtons.value.find( | |
| (button) => button.key === nextKey, | |
| ) | |
| emit('update:modelValue', selectedButton?.modelValue) | |
| }, | |
| }) |
| :aria-label=" | ||
| button.hideLabel && hasLabel(button.label) | ||
| ? String(button.label) | ||
| : undefined | ||
| " | ||
| :title=" | ||
| button.hideLabel && hasLabel(button.label) | ||
| ? String(button.label) | ||
| : undefined | ||
| " |
There was a problem hiding this comment.
Accessible name missing for unlabeled icon buttons (when hideLabel is false).
aria-label/title are only set when button.hideLabel && hasLabel(...). If a consumer passes only an icon (no label, or hideLabel unset), the rendered <button> has no accessible name and no visible text. Consider also emitting aria-label whenever a label exists but is not visually rendered, or warn/require a label when an icon-only configuration is detected.
Use withBase() for custom theme navigation so pull request previews keep the /pr-preview/pr-<number>/ prefix across the home page, sidebar, prev/next navigation, and search results.
Reorder the tsconfig export conditions so the types entry is reachable and merge duplicate blockquote typography rules to avoid duplicate-object-key warnings during the VitePress build.
1 participant
Summary by CodeRabbit
New Features
Documentation
Chores