-
Notifications
You must be signed in to change notification settings - Fork 2.3k
[WEB-3978] chore: cmd k search result redirection improvements #7012
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
[WEB-3978] chore: cmd k search result redirection improvements #7012
Conversation
WalkthroughA centralized state management system for the open/close state of project lists in the command palette sidebar was implemented using MobX. A helper function was introduced to open a project and scroll to its sidebar item with a highlight animation. Related UI components and global styles were updated to utilize these new mechanisms. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CommandPaletteSearchResults
participant openProjectAndScrollToSidebar
participant CommandPaletteStore
participant SidebarProjectsListItem
participant DOM
User->>CommandPaletteSearchResults: Selects a search result
CommandPaletteSearchResults->>openProjectAndScrollToSidebar: Call with projectId
openProjectAndScrollToSidebar->>CommandPaletteStore: toggleProjectListOpen(projectId, true)
openProjectAndScrollToSidebar->>DOM: Scroll to sidebar item, trigger highlight animation
CommandPaletteStore-->>SidebarProjectsListItem: projectListOpenMap updated
SidebarProjectsListItem->>SidebarProjectsListItem: Rerender based on centralized open state
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (2)
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Pull Request Linked with Plane Work Items Comment Automatically Generated by Plane |
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)
web/core/components/command-palette/actions/helper.tsx (1)
1-20
: Well-structured helper function for sidebar navigation.The
openProjectAndScrollToSidebar
function effectively manages both the state change (opening the project list) and the UI feedback (scrolling and highlighting), creating a cohesive user experience as described in the PR objectives.Consider adding error handling for the DOM manipulation operations, and potentially extracting the element ID construction logic to make the function more reusable:
export const openProjectAndScrollToSidebar = (itemProjectId: string) => { if (!itemProjectId) return; // open the project list store.commandPalette.toggleProjectListOpen(itemProjectId, true); // scroll to the element const scrollElementId = `sidebar-${itemProjectId}-JOINED`; const scrollElement = document.getElementById(scrollElementId); // if the element exists, scroll to it if (scrollElement) { setTimeout(() => { + try { scrollElement.scrollIntoView({ behavior: "smooth", block: "start" }); // Restart the highlight animation every time scrollElement.style.animation = "none"; void scrollElement.offsetWidth; scrollElement.style.animation = "highlight 2s ease-in-out"; + } catch (error) { + console.error("Error highlighting sidebar element:", error); + } }); } };web/core/components/command-palette/actions/search-results.tsx (1)
4-4
: Great enhancement to project navigation UX.The integration of MobX observer and the new sidebar scrolling/highlighting functionality provides a better user experience when selecting search results, aligning perfectly with the PR objectives.
Consider using optional chaining for cleaner project ID extraction:
- const itemProjectId = item.project_id || (item.project_ids && item.project_ids[0]) || undefined; + const itemProjectId = item.project_id || item.project_ids?.[0] || undefined;Also applies to: 12-13, 20-20, 44-45
web/core/store/base-command-palette.store.ts (1)
59-77
: Consider usingobservable.map
for dynamic keys
projectListOpenMap
grows dynamically ([projectId] = …
). With a plain object MobX must patch the object each time a new key appears which is slightly less efficient and can surprise whenobserve
/reaction
is used. A dedicatedobservable.map<string, boolean>()
keeps intent explicit and avoids those edge cases.web/core/components/workspace/sidebar/projects-list-item.tsx (1)
200-204
: Side-effect runs only on match – consider symmetrical closeif (URLProjectId === project.id) setIsProjectListOpen(true);When users navigate away from a project, its list stays expanded because there’s no complementary branch that sets it to
false
. Depending on UX expectations this could leave many projects open in long-running sessions.
If you want the sidebar to reflect the current route strictly, add anelse setIsProjectListOpen(false)
.web/core/components/workspace/sidebar/project-navigation.tsx (1)
143-158
:isActive
logic works butpathname.includes()
can over-matchUsing substring matching may flag an item active for unrelated nested paths (e.g.
"/pages"
inside"/pages-settings"
). For stricter checks:import { match } from "path-to-regexp"; // or use Next.js `router` helpers ... const isPathnameActive = match(item.href, { end: false })(pathname);Alternatively compare
pathname.startsWith(item.href)
which is cheap and avoids mid-string collisions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
web/core/components/command-palette/actions/helper.tsx
(1 hunks)web/core/components/command-palette/actions/search-results.tsx
(3 hunks)web/core/components/workspace/sidebar/project-navigation.tsx
(5 hunks)web/core/components/workspace/sidebar/projects-list-item.tsx
(4 hunks)web/core/store/base-command-palette.store.ts
(6 hunks)web/core/store/issue/issue-details/issue.store.ts
(1 hunks)web/styles/globals.css
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
web/core/components/workspace/sidebar/projects-list-item.tsx (2)
web/core/hooks/store/use-command-palette.ts (1)
useCommandPalette
(7-11)web/core/store/router.store.ts (1)
projectId
(85-87)
web/core/store/base-command-palette.store.ts (1)
web/core/store/router.store.ts (1)
projectId
(85-87)
🪛 Biome (1.9.4)
web/core/components/command-palette/actions/search-results.tsx
[error] 44-44: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (3)
web/styles/globals.css (1)
945-955
: Clean animation implementation for sidebar highlighting.The new
highlight
keyframe animation provides a smooth visual effect to draw attention to elements in the sidebar. It transitions from a fully opaque background color to transparent, maintaining a consistent border radius.web/core/store/issue/issue-details/issue.store.ts (1)
309-309
: Good fix for issue identifier registration.Adding the issue identifier to the store immediately after validation ensures it's available before any subsequent operations, preventing potential state inconsistencies between different parts of the application.
web/core/components/workspace/sidebar/projects-list-item.tsx (1)
67-85
: Good move to centralised state, but mind thetoggleProjectListOpen
contractThe component now relies on the store-level
toggleProjectListOpen
. After fixing the contract bug in the store (see comment above) this usage will work as expected. Without the fix,setIsProjectListOpen(false)
may reopen a list that was already closed.
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 (1)
web/core/store/issue/issue.store.ts (1)
63-68
: Consider adding null/undefined checks for projectIdentifier and workItemSequenceIdThe code builds an issue identifier by combining the project identifier and sequence ID, but doesn't handle cases where these values might be undefined or null.
- // add issue identifier to the issuesIdentifierMap - const projectIdentifier = rootStore.projectRoot.project.getProjectIdentifierById(issue?.project_id); - const workItemSequenceId = issue?.sequence_id; - const issueIdentifier = `${projectIdentifier}-${workItemSequenceId}`; - set(this.issuesIdentifierMap, issueIdentifier, issue.id); + // add issue identifier to the issuesIdentifierMap + const projectIdentifier = rootStore.projectRoot.project.getProjectIdentifierById(issue?.project_id); + const workItemSequenceId = issue?.sequence_id; + if (projectIdentifier && workItemSequenceId) { + const issueIdentifier = `${projectIdentifier}-${workItemSequenceId}`; + set(this.issuesIdentifierMap, issueIdentifier, issue.id); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/core/store/issue/issue.store.ts
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (2)
web/core/store/issue/issue.store.ts (2)
10-10
: LGTM: Import for accessing project identifier dataThis import of
rootStore
is necessary to access project identifier information used in the enhanced issue identification system.
63-68
: Good implementation supporting the Command+K search result navigation enhancementThis code elegantly implements the issue identification mapping needed for the enhanced Command+K search result navigation mentioned in the PR objectives. The mapping enables the application to find the right project and issue when a user selects a search result, supporting the new ability to expand the relevant project in the sidebar and highlight it.
Description
This PR improves the redirection behavior from the Cmd+K search results. In addition to redirecting to the selected page, it now expands the corresponding project and highlights it in the app sidebar.
A new utility function, openProjectAndScrollToSidebar, has been added. It accepts a projectId, and can be reused wherever similar behavior is needed.
Type of Change
Media
References
[WEB-3978]
Summary by CodeRabbit