feat: Global search for all scene/external/prefabs/extensions event sheet #8292
feat: Global search for all scene/external/prefabs/extensions event sheet #8292ViktorVovk wants to merge 21 commits into4ian:masterfrom
Conversation
- Implemented methods to set and clear global search results in EventsFunctionsExtensionEditor and EventsEditorContainer. - Enhanced EventsSheetComponent to manage global search results and focus. - Added UI components for displaying highlighted search results across various event renderers. - Introduced a new GlobalEventsSearchEditorContainer for managing global search interactions. - Updated MainFrame to handle navigation and state management for global search.
- Introduced a new SVG icon for the global search functionality. - Implemented a utility function `highlightSearchText` to highlight search terms in various components. - Updated multiple components (Instruction, DefaultField, ObjectField, VariableField, GlobalEventsSearchEditorContainer) to utilize the new highlighting function. - Enhanced the MainFrame to include global search options in the menu and project manager. - Integrated global search event handling for improved user experience.
- Added a check for bad instruction metadata in the Instruction component. - Introduced a fallback rendering for missing instructions using the InstructionMissing component. - Ensured that the small icon filename defaults to an empty string if not available.
|
This looks like a cool addition. I know this is still a draft but from a functional point of view we can think of these ways to discover this feature:
This last point would allow one day to extend this search with a "Search anything" (that could search in events, object properties, behavior properties, extension properties...) (but that's not the point of this PR of course). |
- Introduced the GlobalEventsSearchEditor for managing global search interactions. - Created supporting components including GroupItem, GroupList, and RenderMatchRow for displaying search results. - Implemented context management for search state and navigation using GlobalSearchContext. - Added utility functions for handling search logic and deduplication of event paths. - Updated BaseEditor to streamline navigation to events from global search results.
- Added matchCase parameter to global search methods across various components, including EventsFunctionsExtensionEditor, EventsEditorContainer, and GlobalEventsSearchEditor. - Updated highlightSearchText utility to support case-sensitive searches. - Modified multiple components to utilize the new matchCase feature for improved search accuracy. - Ensured consistent handling of search results and state management in the MainFrame and related containers.
|
HI @4ian. First of all, I would like to thank you for reviewing my PR, even though it is still in Draft. Your comments are truly helpful, as I spent quite some time thinking about the best place to position the project search button. I chose Game Settings because it’s quick to access and easy to find. However, you are absolutely right — the “Search” feature is not really part of the project itself, and to be honest, I was counting on your feedback and suggestions. In my opinion, a good place could be a transition from the scene search to the project-wide search. However, I’m still struggling from a UI/UX perspective with how and where exactly to place a button (or another control) that would switch from the local search to the global search tab. I would really appreciate it if you could provide a rough mockup (PNG) showing how you think the control for switching from local search to the global search tab should be positioned. Additionally, I have already added a menu item under View => Global Search. I would also like to add a hotkey for this feature. I’d be glad if you could suggest which key combination would be the most appropriate. Regarding the context menu, I’m not entirely sure about it yet, since the entire line gets selected, and actions or conditions can sometimes be quite long.
|
- Deleted the unused search_black.svg icon from the project. - Added 'OPEN_GLOBAL_SEARCH' command to the CommandsList for initiating global search. - Defined keyboard shortcut 'CmdOrCtrl+Shift+KeyF' for the global search functionality. - Updated MainFrame and command handlers to support the new global search command. - Adjusted KeyboardShortcuts to prevent conflicts with existing search functionality.
…ld and ObjectField components - Updated the highlightSearchText function calls in DefaultField and ObjectField to consistently use the matchCase parameter for improved case-sensitive search functionality. - Removed unnecessary spanProps variable to streamline the code.
…tem components - Simplified the rendering of search result messages by replacing Trans components with template literals for better readability and performance. - Updated the label for match counts in GroupItem to use string interpolation, enhancing consistency across the search interface.
|
@ViktorVovk This is still in draft, do you want a new review or not yet? :)
Sounds good, I think Ctrl/Cmd+Shift+F is fine (similar to IDEs where Ctrl+F is in the text file, Ctrl+Shift+F is in the whole project/workspace).
Perfect! That's fine to me.
We might not display it in the project manager and just count on people finding it through View and from a link the "local" search (see next point).
I do agree it would be best to find this feature to have the ability to "transition" from local search to this "global search". I just... am not sure how to do this yet! |
- Updated the setGlobalSearchResults method in multiple components (EventsFunctionsExtensionEditor, EventsSheet, EventsEditorContainer, EventsFunctionsExtensionEditorContainer, ExternalEventsEditorContainer) to enforce non-nullable types for focusedEventPath, searchText, and matchCase parameters. - Ensured consistent handling of search text and case sensitivity across the application for improved search functionality.
- Updated the globalSearchMatchCase assignment to directly use the matchCase parameter for improved clarity and consistency in search functionality.
Good day @4ian , Please accept my apologies for the delay. Yes, the PR is ready for review. |
|
No worries! Let me run an AI powered review to see if it catches any obvious issue and we will review it too now. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c737575b76
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
newIDE/app/src/MainFrame/index.js
Outdated
| openEventsFunctionsExtension( | ||
| extensionName || name, | ||
| functionName, | ||
| behaviorName, | ||
| objectName | ||
| ); |
There was a problem hiding this comment.
Select target extension function before navigating match
This branch assumes passing functionName/behaviorName/objectName to openEventsFunctionsExtension is enough to land on the matched function, but openEditorTab only focuses an already-open extension tab and does not reapply extraEditorProps. In that common case, clicking a global-search result from a different function keeps the old function selected, so scrollToEventPath/setGlobalSearchResults run against the wrong events list and the user is taken to an unrelated location.
Useful? React with 👍 / 👎.
newIDE/app/src/MainFrame/index.js
Outdated
| setTimeout(() => { | ||
| const editorKind = | ||
| locationType === 'layout' | ||
| ? 'layout events' | ||
| : locationType === 'external-events' | ||
| ? 'external events' | ||
| : 'events functions extension'; |
There was a problem hiding this comment.
Wait for editor ref before applying global search results
Applying highlights is attempted once via a fixed 450ms timeout; if the destination editor has not mounted yet (slow machine, heavy project, throttled tab), editorRef is still null and this callback exits without retrying, so matches are not highlighted even though navigation was requested. This should wait for/refetch the editor ref (or retry with a bounded loop) instead of relying on a single delay.
Useful? React with 👍 / 👎.
- Introduced a new function to apply search results conditionally based on the current context (extension or standard). - Added logic to ensure the correct function is selected before setting search results, improving the responsiveness of the search feature. - Utilized requestAnimationFrame to defer search result application, allowing for smoother UI updates.
- Added a new prop `searchFocusedEvent` to track the currently focused event during search in the EventsTree. - Updated the SortableEventsTree to utilize `searchFocusedEvent` for improved search match handling. - Introduced a global search retry mechanism in MainFrame to manage search result application more effectively, ensuring smoother user experience.
4ian
left a comment
There was a problem hiding this comment.
First review pass, only nitpickings for now! We need to go deeper in the review for:
- The search logic itself (just quickly went through it)
- The opening in MainFrame (MainFrame is pretty big already, so I tend to try to move any logic that is added to it to hooks or helper functions so MainFrame is long but not complex)
We will get back to you with more comments as we dig more into the search feature itself :)
| @@ -0,0 +1,6 @@ | |||
| // @flow | |||
There was a problem hiding this comment.
Big nitpicking from my side but we usually don't do in the codebase such "index re-exporting type/components" files, because they add more noise than actual encapsulation.
newIDE/app/src/MainFrame/EditorContainers/GlobalSearch/RenderMatchRow.js
Outdated
Show resolved
Hide resolved
- Introduced a new type `EventPath` to standardize event path representation as an array of numbers. - Updated multiple components (e.g., EventsSheet, MainFrame, GlobalSearch) to utilize `EventPath` for improved type safety and consistency. - Modified relevant functions and props to accept and return `EventPath`, enhancing clarity in event navigation and search functionalities.
…ex file - Updated the import path for `GlobalEventsSearchEditor` to reflect its new location within the directory structure. - Deleted the now unnecessary `index.js` file in the `GlobalSearch` directory, streamlining the codebase.
- Replaced the `onGlobalSearchWillClose` callback with `onEditorTabClosing` to streamline tab closing behavior. - Updated the `renderTabs` function to ensure tooltips are cleared and the correct editor tab is closed. - Enhanced the `onEditorTabClosing` function to clear global search highlights when a global search tab is closed, improving user experience.
- Replaced `RenderMatchRowList` with `SearchMatchRowList` to enhance clarity in match rendering. - Introduced `SearchMatchRow` component for improved match display and interaction. - Deleted obsolete `RenderMatchRowList` file, streamlining the codebase.
- Consolidated global and local search state management into a single `searchHighlight` object for improved clarity and efficiency. - Updated state initialization and search result handling to accommodate the new structure, enhancing the search functionality. - Simplified the rendering logic to utilize the new `searchHighlight` state, ensuring consistent behavior across search operations.
- Introduced a new type import for `EditorTab` from the `EditorTabsHandler` to enhance type safety and clarity in the MainFrame component. - This addition prepares the codebase for future enhancements related to editor tab management.
- Introduced methods for navigating search results (_goToNextFromGlobalSearch and _goToPreviousFromGlobalSearch) to improve user experience during search operations. - Updated state management to handle search highlights more effectively, allowing for seamless transitions between global and local searches. - Enhanced the SearchPanel component to synchronize external search state, ensuring that initial search parameters are correctly reflected in the UI.
|
Hi @4ian , Thank you for the review — I’ve done my best to address all the comments you left. Additionally, I implemented synchronization between global and local search when navigating from global search results to an event sheet (commit The idea was that when we navigate from the global search results to an event sheet tab, the local search form should automatically open and receive the current search state. This allows us to leverage the advantages of the local search, such as navigating between results and potentially replacing matched entries. Secondly, this resolves a logic conflict we previously had — when there were existing local search results and we navigated to the same event sheet with completely different results coming from the global search, it raised questions:
|
4ian
left a comment
There was a problem hiding this comment.
Thanks for the updates! Looks better for everything. Added a comment about translation. Another pass to do on our side to check how the search is made and we should be able to merge this next week :)
| size="small" | ||
| variant="outlined" | ||
| color="secondary" | ||
| label={totalMatches === 1 ? '1 match' : `${totalMatches} matches`} |
There was a problem hiding this comment.
Use translation component for all strings when a React.Node is supported:
| label={totalMatches === 1 ? '1 match' : `${totalMatches} matches`} | |
| label={totalMatches === 1 ? <Trans>1 match</Trans> : <Trans>{totalMatches} matches</Trans>} |
(and in the case where a string is expected, then you either use
i18n._(t`your text`)
or just pass the text with the t macro and let the component call i18n._ (in which case the type should be MessageDescriptor for the prop))
| @@ -0,0 +1,6 @@ | |||
| // @flow | |||
This is a great side effect! Can you confirm if the previous "conflict between global search + local search" is now solved or if there is still some arbitrage to do in the interface between them? |
|
Yes, this has now been fully resolved and implemented. |
…tsSearchEditor and GroupItem components - Wrapped match count text in <Trans> components to support localization in the GlobalEventsSearchEditor. - Updated label for match count in GroupItem to use <Trans> for consistent internationalization across components.
Hello. I would like to present my PR in which I implemented a global project search feature.
Currently, in large projects with many scenes and external events, it can be quite time-consuming to quickly find specific items such as actions, conditions, or variables. For this reason, a global search functionality has been implemented.
It is designed as a separate tab panel that contains a search form and search results grouped by scenes. When clicking a search result entry, the editor automatically navigates to the corresponding tab (scene event sheet or external events), where the matches are highlighted.
I would like to share a few screenshots to demonstrate how it currently looks:
