-
Notifications
You must be signed in to change notification settings - Fork 326
feat(search): Add mini search box expansion and retraction hook callback API #3664
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
Conversation
WalkthroughAdds PC-only Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant PC as Search (PC - mini)
participant RL as Renderless
participant V as Vue Emits
U->>PC: Click search icon/input
PC->>RL: searchClick()
alt mini mode & collapsed
RL->>RL: state.collapse = false
RL->>V: emit("expand")
else
RL->>V: emit("search")
end
U->>PC: Click outside
PC->>RL: clickOutside()
alt mini mode & empty value
RL->>RL: state.collapse = true
RL->>V: emit("collapse")
else
RL-->>PC: no-op
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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. ✨ 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: 1
🧹 Nitpick comments (5)
examples/sites/demos/pc/app/search/webdoc/search.js (1)
115-121
: Docs: tighten EN copy and fix spacing around code tagsGood switch to template literals and the new expand/collapse bullets. The EN paragraph has awkward phrasing and missing spaces around code tags. Suggested copy below keeps it concise and consistent with component terminology (“Enter” instead of “carriage return”).
- 'en-US': `<div class="tip custom-block"> - Set carriage return to trigger search event through<code>is-enter-search</code>, and<code>search</code>to listen for search events; <br />\n - Monitor the search value change event when the input box loses focus through<code>change</code>, and monitor the real-time search value change event through<code>input</code>; <br />\n - Monitor the search type selection event through<code>select</code>; <br />\n - Monitor mini search box expansion events through<code>expand</code>; <br />\n - Monitor the mini search box collapse event through<code>collapse</code>.</div>` + 'en-US': `<div class="tip custom-block"> + Use <code>is-enter-search</code> to trigger a search on Enter, and listen with <code>search</code>.<br />\n + Listen for value changes on blur with <code>change</code>, and for real-time changes with <code>input</code>.<br />\n + Listen for type selection with <code>select</code>.<br />\n + Listen for mini search box expansion with <code>expand</code>.<br />\n + Listen for mini search box collapse with <code>collapse</code>.</div>`Also applies to: 122-126
examples/sites/demos/pc/app/search/events.vue (1)
18-21
: Nice addition: mini-mode expand/collapse demoWires the new events clearly. Consider adding a brief tip text so users know to click the search icon to expand and click outside with an empty value to collapse, but it’s optional.
packages/renderless/src/search/index.ts (1)
60-63
: UX: focus the input when expanding mini modeWhen mini is collapsed and the search icon is clicked, you expand but don’t focus the input. Focusing improves usability and mirrors common patterns.
Apply this small refactor (requires passing vm/nextTick into searchClick and updating the caller in vue.ts):
-export const searchClick = - ({ emit, props, state }: Pick<ISearchRenderlessParams, 'emit' | 'props' | 'state'>) => +export const searchClick = + ({ emit, props, state, vm, nextTick }: Pick<ISearchRenderlessParams, 'emit' | 'props' | 'state' | 'vm' | 'nextTick'>) => (event: Event) => { event.preventDefault() if (props.mini && state.collapse) { state.collapse = false emit('expand') + nextTick(() => (vm.$refs.input as HTMLInputElement | undefined)?.focus?.()) } else { emit('search', state.searchValue, state.currentValue) } }And in packages/renderless/src/search/vue.ts:
- searchClick: searchClick({ emit, props, state }), + searchClick: searchClick({ emit, props, state, vm, nextTick }),examples/sites/demos/apis/search.js (2)
221-234
: Polish EN/ZH copy and mark version in desc for consistency with existing “[新增]” style.Other entries (e.g., input) embed the “added in” version in desc; here you introduced meta.stable. To keep docs uniform, append the version to desc as well and tweak EN phrasing (“collapses” reads better than “is retracted”). Keep meta if your doc-gen consumes it.
desc: { - 'zh-CN': 'mini模式的搜索框收回时触发的回调函数', - 'en-US': 'The callback function triggered when the search box in mini mode is retracted' + 'zh-CN': '迷你模式下搜索框收回时触发的回调函数【3.26.0新增】', + 'en-US': 'Callback function emitted when the mini search box collapses. [Added in 3.26.0]' },
235-248
: Mirror the copy updates for expand.Apply the same consistency tweaks as collapse.
desc: { - 'zh-CN': 'mini模式的搜索框展开时触发的回调函数', - 'en-US': 'The callback function triggered when the search box in mini mode expands' + 'zh-CN': '迷你模式下搜索框展开时触发的回调函数【3.26.0新增】', + 'en-US': 'Callback function emitted when the mini search box expands. [Added in 3.26.0]' },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
examples/sites/demos/apis/search.js
(1 hunks)examples/sites/demos/pc/app/search/events.vue
(2 hunks)examples/sites/demos/pc/app/search/webdoc/search.js
(1 hunks)packages/renderless/src/search/index.ts
(2 hunks)packages/renderless/src/search/vue.ts
(1 hunks)packages/vue/src/search/src/pc.vue
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/renderless/src/search/vue.ts (1)
packages/renderless/src/search/index.ts (1)
clickOutside
(77-91)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (5)
packages/renderless/src/search/vue.ts (1)
106-106
: LGTM: emit is correctly forwarded to clickOutsidePassing emit into clickOutside aligns with the updated signature and enables public event emission from renderless. No further changes needed here.
packages/vue/src/search/src/pc.vue (1)
119-119
: Expose new events (expand/collapse) — goodEmits list now includes expand and collapse, matching renderless behavior and demos/docs.
examples/sites/demos/pc/app/search/events.vue (1)
65-69
: Handlers read wellModal messages for expand/collapse are straightforward and consistent with other examples.
examples/sites/demos/apis/search.js (2)
221-248
: Events added are well-scoped and consistent (PC-only, mini mode).Naming, placement in the events array, and void payloads align with existing API patterns. Good addition.
229-231
: Confirm event wiring; verify doc-gen support formeta.stable
Events emission and demo wiring have been confirmed in the codebase, but there’s no obvious reference to
meta.stable
in the doc-generator pipelines—please ensure your tooling picks up this field.• packages/vue/src/search/src/pc.vue (line 119): declares
expand
andcollapse
inemits
• packages/renderless/src/search/index.ts (lines 62, 88): callsemit('expand')
andemit('collapse')
• packages/renderless/src/search/vue.ts & index.ts:clickOutside
is wired to the DOM listener as expected
• examples/sites/demos/pc/app/search/events.vue (lines 20, 65–69):<tiny-search mini @expand="handleExpand" @collapse="handleCollapse">
wired correctly
• examples/sites/demos/pc/app/search/webdoc/search.js (lines 119–126): docs include<code>expand</code>
and<code>collapse</code>
entriesNext steps:
- Verify that your documentation generator recognizes and surfaces the
meta.stable
property fromexamples/sites/demos/apis/search.js
.- If it doesn’t, update the doc-gen configuration or parsing logic to include
meta.stable
in the generated API reference.
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: 1
🧹 Nitpick comments (4)
examples/sites/demos/pc/app/search/events.spec.ts (4)
11-13
: Replace brittle modal count check with an assertion on the new expand eventCounting all elements with
.tiny-modal
is fragile and couples the test to incidental UI details. Since this PR introduces an explicit expand event that shows a TinyModal with '展开', assert that instead. This both validates the new behavior and reduces flakiness.Apply this diff:
await button.click() - await expect(modal).toHaveCount(2) + // Assert the expand event is surfaced via TinyModal + await expect(modal.getByText('展开')).toBeVisible()
38-38
: Avoid fixed 100 ms waits; assert the intended outcome insteadA hardcoded
timeout: 100
makes the test flaky across environments. Prefer an assertion that waits for the element to become visible.Apply this diff:
- await modalContent.waitFor({ state: 'attached', timeout: 100 }) + await expect(modalContent).toBeVisible()
3-39
: Cover the new collapse event explicitlyThe PR adds both 'expand' and 'collapse' events. This spec only validates expand (indirectly). Add an explicit check for '收回' by clicking outside after expansion, ideally as a separate test to avoid interfering with the rest of this flow.
Example new test to add to this file:
test('mini 搜索框 expand/collapse 事件', async ({ page }) => { page.on('pageerror', (exception) => expect(exception).toBeNull()) await page.goto('search#events') const modal = page.locator('.tiny-modal') const button = page.locator('.tiny-search__input-btn > a').first() await button.click() await expect(modal.getByText('展开')).toBeVisible() // Click outside to trigger collapse await page.mouse.click(0, 0) await expect(modal.getByText('收回')).toBeVisible() })If clicking at (0,0) is unreliable in CI, consider clicking a known outside container (e.g.,
#app
) or exposing a stable "outside" area with a data-testid.
7-10
: Prefer role-/text-based or data-testid selectors over structural/nth selectorsSelectors like
.tiny-search__input-btn > a
.first() andinput.nth(1)
/nth(2)
are brittle against DOM refactors. UsinggetByRole
,getByPlaceholder
, ordata-testid
improves stability and readability.Examples:
const button = page.getByRole('button', { name: /搜索|Search/i })
const input = page.getByPlaceholder('请输入内容')
(or use a data-testid on each textbox)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
examples/sites/demos/pc/app/search/events.spec.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
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: 0
🧹 Nitpick comments (2)
examples/sites/demos/pc/app/search/slot-prefix-suffix.spec.ts (2)
24-24
: Make the accessible-name match resilient to quote variations.The literal Chinese curly quotes in the name can differ by locale, fonts, or authoring. Prefer a regex that tolerates straight or curly quotes (or absence) to avoid flaky matches.
Apply this change:
- await page.getByRole('button', { name: '点击切换为“禁用状态”' }).click() + await page.getByRole('button', { name: /点击切换为["“”]?禁用状态["“”]?/ }).click()
26-27
: Assert expected number of search widgets for clearer failures and future-proofing.If the demo page ever adds more .tiny-search instances, the loop will still pass but not tell you if unexpected ones appeared. Add an explicit count assertion before iterating.
- const searchLocators = await page.locator('.tiny-search').all() + await expect(page.locator('.tiny-search')).toHaveCount(2) + const searchLocators = await page.locator('.tiny-search').all()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
examples/sites/demos/pc/app/search/slot-prefix-suffix.spec.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (1)
examples/sites/demos/pc/app/search/slot-prefix-suffix.spec.ts (1)
24-24
: Good move to role-based locator — more resilient and a11y-friendly.Switching to getByRole improves test robustness and aligns with Playwright best practices.
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?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Documentation
Tests