refactor: issue list modal refactor#6702
Conversation
WalkthroughThe changes update the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant M as ExistingIssuesListModal
participant C as Custom Search Service Callback
participant P as projectService.projectIssuesSearch
U->>M: Trigger search
M->>M: Check if workItemSearchServiceCallback is provided
alt Custom Callback Provided
M->>C: Invoke custom search with parameters
else Fallback Search
M->>M: Check if projectId is defined
alt projectId exists
M->>P: Call projectIssuesSearch with parameters
else
M-->>U: No search action executed
end
end
Note over M,U: Update modal with search results
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ 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:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
web/core/components/core/modals/existing-issues-list-modal.tsx (3)
92-111: Simplify conditional search service logicThe current approach works but can be made clearer by using early returns or conditional assignment patterns.
- const handleSearch = () => { - if (!isOpen || !workspaceSlug) return; - setIsLoading(true); - const searchService = - workItemSearchServiceCallback ?? - (projectId - ? projectService.projectIssuesSearch.bind(projectService, workspaceSlug?.toString(), projectId?.toString()) - : undefined); - if (!searchService) return; - searchService({ - search: debouncedSearchTerm, - ...searchParams, - workspace_search: isWorkspaceLevel, - }) + const handleSearch = () => { + if (!isOpen || !workspaceSlug) return; + + // Return early if no search method is available + if (!workItemSearchServiceCallback && !projectId) return; + + setIsLoading(true); + + // Determine which search service to use + const searchService = workItemSearchServiceCallback ?? + projectService.projectIssuesSearch.bind(projectService, workspaceSlug, projectId); + + searchService({ + search: debouncedSearchTerm, + ...searchParams, + workspace_search: isWorkspaceLevel, + })Also note that the
.toString()calls are unnecessary as TypeScript will handle the string conversion automatically when needed.
119-121: Consider search behavior on modal closeThe dependency array includes
isOpen, which meanshandleSearch()will be called both when the modal opens and closes. When the modal is closing, the search is still triggered but has no visible effect since the modal is disappearing.Consider adding a condition to skip the search operation when the modal is closing:
useEffect(() => { + if (!isOpen) return; handleSearch(); }, [debouncedSearchTerm, isOpen, isWorkspaceLevel, projectId, workspaceSlug]);
232-242: Translatable search text still pendingThere's a TODO comment about translating the search results text that isn't addressed in this PR.
Consider updating this section to use the translation framework:
{searchTerm !== "" && ( <h5 className="mx-2 text-[0.825rem] text-custom-text-200"> - Search results for{" "} - <span className="text-custom-text-100"> - {'"'} - {searchTerm} - {'"'} - </span>{" "} - in project: + {t("search.results_for", { searchTerm })}{" "} + {isWorkspaceLevel ? t("search.in_workspace") : t("search.in_project")} </h5> )}This assumes you add the appropriate translation keys to your i18n files.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/core/components/core/modals/existing-issues-list-modal.tsx(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (4)
web/core/components/core/modals/existing-issues-list-modal.tsx (4)
27-27: Good enhancement of component flexibility!Making
projectIdoptional allows the component to be used in more contexts. This change aligns well with the addedworkItemSearchServiceCallbackprop that can be used whenprojectIdis unavailable.
34-35: Well-designed extension of the component APIThe added props enhance the component's reusability by:
- Supporting pre-selected work items with
selectedWorkItems- Allowing custom search functionality with
workItemSearchServiceCallbackThis matches the PR objectives of making the modal reusable with any search function.
113-117: Correctly handles pre-selected work itemsGood implementation of
useEffectto update the selected issues based on external pre-selections when the modal opens or when the pre-selections change.
67-72: Consider persistence of selected issuesWhen the modal closes,
handleClose()resetsselectedIssues. With the newselectedWorkItemsprop, you may want to reconsider this behavior.If users frequently reopen the modal expecting their previous selections to persist, the current implementation would lose those selections. Consider whether:
- Selected issues should persist between modal openings
- Selected issues should reset only when explicitly requested
- The current behavior is in fact correct according to your requirements
The answer depends on the expected UX flow of your application.
* fix: issue stats refactor (#6705) * fix: issue stats refactor * fix: refactor * fix: ui color * fix: translation key * refactor: issue list modal refactor (#6702) --------- Co-authored-by: Akshita Goyal <36129505+gakshita@users.noreply.github.com> Co-authored-by: Vamsi Krishna <46787868+vamsikrishnamathala@users.noreply.github.com> Co-authored-by: sriram veeraghanta <veeraghanta.sriram@gmail.com>
3 participants
Description
This update refactors the work items search modal, enabling it to be used with any search function.
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit