feat(tabs): add header-only #3638
Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... 📒 Files selected for processing (4)
WalkthroughAdded a boolean Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Tabs
participant TabItem
User->>Tabs: enable headerOnly (prop)
Tabs->>Tabs: state.headerOnly = true
Tabs->>TabItem: set rootTabs.headerOnly = true
TabItem->>TabItem: skip rendering pane content
User->>Tabs: disable headerOnly
Tabs->>TabItem: render pane content as normal
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Poem
Tip CodeRabbit can enforce grammar and style rules using `languagetool`.Configure ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (9)
packages/renderless/types/tabs.type.ts (1)
36-36: Add brief JSDoc and platform noteDocument what headerOnly controls and that it’s intended for PC mode to aid discoverability in editors.
packages/vue/src/tabs/src/index.ts (1)
73-75: Prop looks fine; add a short comment and/or explicit default for clarityBoolean props default to false implicitly. Adding a small comment (PC-only) and/or default: false can make intent obvious to readers.
// 只渲染头部 - headerOnly: Boolean + // PC only: when true, only render tab headers; content panels are omitted + headerOnly: { + type: Boolean, + default: false + }examples/sites/demos/apis/tabs.js (1)
297-309: Consider adding “since version” metadataIf your docs pattern supports it, noting the version this prop was introduced in helps consumers track availability.
packages/vue/src/tab-item/src/pc.vue (2)
14-14: Optional: extract condition into a computed for readabilityThe in-template boolean is getting long. A computed like shouldRenderPanel improves readability and reuse.
In script:
const shouldRenderPanel = computed( () => !state.rootTabs.headerOnly && (!props.lazy || state.loaded || state.active) )In template:
<div v-if="shouldRenderPanel" v-show="state.active" ...>
14-14: Tests missing for new modeAdd unit tests to assert no tabpanel elements render when header-only is true, and that tab header interactions still emit click/change events as expected.
I can scaffold @vue/test-utils + vitest tests if helpful.
examples/sites/demos/pc/app/tabs/webdoc/tabs.js (1)
279-281: Fix typo and improve en-US phrasing/capitalization
- Extra ">" inside the code tag.
- Improve phrasing; capitalize "Only" in the demo name for consistency.
Apply:
- 'en-US': 'Use <code>>header-only</code> header only.' + 'en-US': 'Use <code>header-only</code> to display only the header.'And optionally:
- 'en-US': 'Header only' + 'en-US': 'Header Only'Also applies to: 276-276
examples/sites/demos/pc/app/tabs/header-only.vue (3)
2-2: Avoid passing a position that will be ignored in header-only modeSince the component currently forces
positionto 'top' whenheader-onlyis set, remove the:positionbinding (or change the component to respect it).Apply:
- <tiny-tabs tab-style="card" style="width: 500px" :position="position" header-only> + <tiny-tabs tab-style="card" style="width: 500px" header-only>And drop the unused
positionfrom data:- position: 'left',Also applies to: 19-20
9-9: Use plain JS for the script blockNo JSX is used; prefer
lang="js"for clarity.-<script lang="jsx"> +<script lang="js">
32-33: Remove unused data property
tabIndexis defined but unused.- ], - tabIndex: 2 + ]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
examples/sites/demos/apis/tabs.js(1 hunks)examples/sites/demos/pc/app/tabs/header-only.vue(1 hunks)examples/sites/demos/pc/app/tabs/webdoc/tabs.js(1 hunks)packages/renderless/src/tabs/vue.ts(1 hunks)packages/renderless/types/tabs.type.ts(1 hunks)packages/vue/src/tab-item/src/pc.vue(1 hunks)packages/vue/src/tabs/src/index.ts(1 hunks)packages/vue/src/tabs/src/pc.vue(3 hunks)
🔇 Additional comments (7)
packages/renderless/types/tabs.type.ts (1)
36-36: Type addition for headerOnly is reasonableOptional boolean on ITabsState aligns with the new prop and existing optional fields (e.g., separator). No correctness concerns.
packages/vue/src/tabs/src/index.ts (1)
72-72: Scope check: panelWidth included in this PRpanelWidth appears alongside headerOnly here. If it’s unrelated to “header-only tabs,” consider splitting to keep PR scope focused and ease tracing changes. If intentional, ensure API docs and demos cover it (they seem to).
examples/sites/demos/apis/tabs.js (1)
297-309: API surface addition LGTMThe header-only prop is documented with clear zh-CN/en-US descriptions, pc-only mode, and demo reference.
packages/vue/src/tab-item/src/pc.vue (1)
14-14: Conditional omit of tab panels aligns with header-only behaviorv-if gate prevents content mount in header-only mode. This matches the feature goal.
examples/sites/demos/pc/app/tabs/webdoc/tabs.js (1)
273-283: Demo entry linkage LGTMdemoId, description, and codeFiles correctly point to header-only.vue; matches the new feature intent.
packages/vue/src/tabs/src/pc.vue (2)
55-57: Prop addition LGTMNew boolean prop
headerOnlyis correctly exposed in the component’s props list.
96-96: Confirm headerOnly orientation UX
The PC implementation currently hard‐codes the tab header to the top whenheaderOnlyis true—ignoring any consumer‐providedposition(e.g. your demo usesleft). We need to decide whether:
- Header‐only must always render at top. In that case, update demos/docs to remove or call out
positionwhenheaderOnlyis applied.- Headers should respect the
positionprop even in header‐only mode. Then remove the override so user orientation is honored.Locations to review:
- packages/vue/src/tabs/src/pc.vue:
Changing this is sufficient because
- Line ~96:
- const position = headerOnly ? 'top' : this.position + const position = this.positionTabNavPC readsstate.rootTabs.position(from this localposition)—no extra prop forwarding is required.Please confirm the intended UX and update code/docs accordingly.
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Tabs很多时候会作为状态控件使用。
Tabs are often used as status controls.
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Documentation
Style