Conversation
Enable collapsible settings sections and allow customizing the sidebar search placeholder. Updates include: new SettingsProps.searchPlaceholder, pass-through from Settings -> SettingsSidebar, and placeholder on the search input. SettingsSection now supports collapsible/collapsed in types, manages open state, toggles via header button with aria attributes, renders a ChevronDown icon, and prevents tooltip/doc link clicks from toggling collapse. Also import useState where needed and add .vscode to .gitignore.
Co-authored-by: Copilot <copilot@github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds configurable sidebar search and collapsible sections to Settings, migrates notice network calls to ChangesSettings core & UI
Notices & WordPress integration
Repo & editor config
Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsRoot as Settings (root)
participant Sidebar as SettingsSidebar
participant Content as SettingsContent
User->>Sidebar: type search (uses searchPlaceholder)
Sidebar->>SettingsRoot: emit search query / update filter state
SettingsRoot->>Content: forward search/filter state
User->>Content: click or press Enter/Space on section header
Content->>Content: toggle open state (collapsible/collapsed)
Content-->>User: show/hide section body (aria-expanded updated)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/settings/settings-content.tsx (1)
273-283:⚠️ Potential issue | 🟠 MajorAdd keyboard event propagation stop to the tooltip trigger.
The
stopPropagation()on the span'sonClickonly prevents mouse clicks from bubbling; it doesn't handle keyboard events. When the button is focused and activated via Enter/Space, the keydown event bubbles to the parent heading and triggersonHeadingKeyDown, which callspreventDefault()and breaks the button's keyboard behavior. AddonKeyDown={(e) => e.stopPropagation()}to the span to prevent keyboard event bubbling to the parent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/settings/settings-content.tsx` around lines 273 - 283, The tooltip trigger only stops click propagation, so keyboard activations still bubble to the parent and break behavior; update the TooltipTrigger span (the inline-flex span inside TooltipTrigger used with <Info />) to also stop keyboard events by adding an onKeyDown handler that calls e.stopPropagation(), ensuring keyboard events do not reach parent handlers such as onHeadingKeyDown and preserving expected keyboard activation for the tooltip trigger.src/components/wordpress/AdminNotice.tsx (1)
214-220:⚠️ Potential issue | 🟡 MinorPreserve a minimum height for the notice card.
Removing
min-h-40makes the carousel collapse to each notice’s content height, which will cause the notice/navigation area to jump as users move between short and tall notices.♻️ Restore the minimum-height guard
- <div - className={cn( - 'overflow-hidden rounded-md flex border-l-4 bg-white p-2 transition-all items-start duration-300 ease-in-out md:p-4', - noticeAccentClass[activeNotice.type] - )}> + <div + className={cn( + 'overflow-hidden rounded-md flex min-h-40 border-l-4 bg-white p-2 transition-all items-start duration-300 ease-in-out md:p-4', + noticeAccentClass[activeNotice.type] + )}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/wordpress/AdminNotice.tsx` around lines 214 - 220, The notice card is collapsing between carousel items because the min-height Tailwind utility was removed; restore a minimum height by re-adding the min-h-40 (or equivalent) utility to the notice container class list where the className is built (the div using cn(..., noticeAccentClass[activeNotice.type])) so the carousel and navigation area remain stable; this change touches the same element that has onMouseEnter={pauseAutoSlide} and onMouseLeave={resumeAutoSlide}.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 70: The `@wordpress/`* packages listed in dependencies (e.g.,
"@wordpress/api-fetch", "@wordpress/components", "@wordpress/dataviews",
"@wordpress/hooks") should be moved to peerDependencies so consumers provide
them at runtime; update package.json by removing these entries from
"dependencies" and adding equivalent semver entries under "peerDependencies"
(and optionally add them to "devDependencies" to keep them available for local
development/testing), ensuring version ranges match the previous values.
In `@src/styles.css`:
- Around line 256-257: Update the role-based selector so elements with
aria-disabled="true" are treated as disabled: change the selector that currently
reads button:not(:disabled), [role="button"]:not(:disabled) to also exclude
[aria-disabled="true"] (i.e. add :not([aria-disabled="true"]) to the
[role="button"] clause) so pseudo-buttons with aria-disabled do not receive
enabled styles like pointer cursor.
---
Outside diff comments:
In `@src/components/settings/settings-content.tsx`:
- Around line 273-283: The tooltip trigger only stops click propagation, so
keyboard activations still bubble to the parent and break behavior; update the
TooltipTrigger span (the inline-flex span inside TooltipTrigger used with <Info
/>) to also stop keyboard events by adding an onKeyDown handler that calls
e.stopPropagation(), ensuring keyboard events do not reach parent handlers such
as onHeadingKeyDown and preserving expected keyboard activation for the tooltip
trigger.
In `@src/components/wordpress/AdminNotice.tsx`:
- Around line 214-220: The notice card is collapsing between carousel items
because the min-height Tailwind utility was removed; restore a minimum height by
re-adding the min-h-40 (or equivalent) utility to the notice container class
list where the className is built (the div using cn(...,
noticeAccentClass[activeNotice.type])) so the carousel and navigation area
remain stable; this change touches the same element that has
onMouseEnter={pauseAutoSlide} and onMouseLeave={resumeAutoSlide}.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7e3d0f5a-91fe-4cb6-9197-f5af3372e2e5
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
.gitignorepackage.jsonsrc/components/settings/index.tsxsrc/components/settings/settings-content.tsxsrc/components/settings/settings-sidebar.tsxsrc/components/settings/settings-types.tssrc/components/top-bar.tsxsrc/components/wordpress/AdminNotice.tsxsrc/components/wordpress/style.csssrc/hooks/use-notices.tssrc/index.tssrc/styles.csswebpack.config.js
…fault values in settings schema
Co-authored-by: Copilot <copilot@github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/settings/settings-sidebar.tsx`:
- Around line 163-178: When `searchable` flips to false the hidden `search`
state must be cleared so the menu isn't silently filtered; add logic in the
SettingsSidebar component to reset `search` (call setSearch('')) whenever
`searchable` becomes false (e.g. useEffect(() => { if (!searchable)
setSearch('') }, [searchable])), or alternatively ignore the stored search by
passing an empty string when rendering the Input if `!searchable`; update
references to `search`, `setSearch`, and the `Input` render so the menu always
shows unfiltered items when `searchable` is disabled.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cbfdd02c-a4b2-45b6-a2d8-113aafdff8d0
📒 Files selected for processing (6)
src/components/settings/Settings.stories.tsxsrc/components/settings/index.tsxsrc/components/settings/settings-sidebar.tsxsrc/components/settings/settings-types.tssrc/components/ui/sidebar.tsxsrc/components/wordpress/layout-menu.tsx
✅ Files skipped from review due to trivial changes (2)
- src/components/wordpress/layout-menu.tsx
- src/components/ui/sidebar.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/settings/settings-types.ts
Comment on lines
+163
to
+178
| {searchable && ( | ||
| <div className="shrink-0 p-2 px-4"> | ||
| <div className="relative"> | ||
| <Search className="text-muted-foreground pointer-events-none absolute left-2.5 top-1/2 size-4 -translate-y-1/2" /> | ||
| <Input | ||
| type="search" | ||
| value={search} | ||
| onChange={(e) => setSearch(e.target.value)} | ||
| className="h-8 pl-8" | ||
| placeholder={searchPlaceholder} | ||
| aria-label="Search settings" | ||
| data-testid="settings-search" | ||
| /> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Clear hidden search state when searchable is turned off.
If a query exists and searchable later becomes false, the menu stays filtered while the input is hidden, so users can’t clear it. Reset or ignore search when search is disabled.
💡 Suggested fix
-import { useMemo, useState } from 'react';
+import { useEffect, useMemo, useState } from 'react';
@@
const [search, setSearch] = useState('');
+
+ useEffect(() => {
+ if (!searchable) {
+ setSearch('');
+ }
+ }, [searchable]);
@@
- const q = search.trim().toLowerCase();
+ const q = searchable ? search.trim().toLowerCase() : '';
if (!q) return items;
@@
- }, [items, search, searchIndex]);
+ }, [items, search, searchIndex, searchable]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/settings/settings-sidebar.tsx` around lines 163 - 178, When
`searchable` flips to false the hidden `search` state must be cleared so the
menu isn't silently filtered; add logic in the SettingsSidebar component to
reset `search` (call setSearch('')) whenever `searchable` becomes false (e.g.
useEffect(() => { if (!searchable) setSearch('') }, [searchable])), or
alternatively ignore the stored search by passing an empty string when rendering
the Input if `!searchable`; update references to `search`, `setSearch`, and the
`Input` render so the menu always shows unfiltered items when `searchable` is
disabled.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Style
Chores