-
Notifications
You must be signed in to change notification settings - Fork 319
feat(tabs): add header-only #3638
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
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 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
position
to 'top' whenheader-only
is set, remove the:position
binding (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
position
from 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
tabIndex
is 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
headerOnly
is 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 whenheaderOnly
is 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
position
whenheaderOnly
is applied.- Headers should respect the
position
prop 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.positionTabNav
PC readsstate.rootTabs.position
(from this localposition
)—no extra prop forwarding is required.Please confirm the intended UX and update code/docs accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
请按照建议更改 @liangguanhui0117
type: 'boolean', | ||
defaultValue: 'false', | ||
desc: { | ||
'zh-CN': '当 header-only 为 true 时,页签内容不再渲染,并且 position 将被设置为固定值 top', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
建议描述更改为:当 header-only 为 true 时,页签内容不再渲染;此时position 值只能为 top
但更好的方式应该是支持所有的position值,只有top不太合理
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
那我把所有position模式,都支持下
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