Skip to content
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

[Editor] Hide Search Results by default and show it on first search. #88465

Merged
1 commit merged into from
Mar 25, 2024

Conversation

AeioMuch
Copy link
Contributor

@AeioMuch AeioMuch commented Feb 17, 2024

Bugsquad edit: fixes godotengine/godot-proposals#9114
This PR aims to implement this proposal : godotengine/godot-proposals#9114

This add the find_in_files_button only when the first search to find in files is made so that it is placed in the last position.
This shows the find_in_files_button only when the first search to find in files is made and push it in the last position.
It also add a Close button that hide the panel and its button when the user want it. I choose to keep the Search Results open even when switching away from the Script editor since the Close button is now available to hide it at will.
Also switch to the script editor when the user click on a line found.

This is my first PR on the editor and found the above proposal was fitting to be an easy first attempt. I tried to avoid doing stupid things and from my test I did not find any issue, but let me know if I did something stupid...

@AeioMuch AeioMuch requested a review from a team as a code owner February 17, 2024 21:07
@Calinou Calinou added this to the 4.x milestone Feb 17, 2024
@AeioMuch AeioMuch force-pushed the search_results_button branch 2 times, most recently from e86ce24 to fe96e6a Compare February 17, 2024 22:13
@passivestar
Copy link
Contributor

Works as expected, implements everything from the proposal

Didn't look at the code

@AeioMuch AeioMuch force-pushed the search_results_button branch from fe96e6a to 9bd64ff Compare February 18, 2024 09:15
@AeioMuch
Copy link
Contributor Author

AeioMuch commented Feb 18, 2024

Since one of the build failed because of memory leak I pushed a new commit where I've tried to do it a bit differently : The Search Results button is created and added inside of the class constructor then hidden; and once a search is done it is pushed at the end and showed. I have also make it so that when you click on a line found, the main screen editor is set to the "Script" editor.

I hope this change will help with those apparent memory leaks, since my guess was that the button needed to be added in the constructor to be properly cleaned afterwards, and that's how it was done before. If that was not the issue then honestly I don't know why, and sorry for the inconvenience!

@passivestar
Copy link
Contributor

I have also make it so that when you click on a line found, the main screen editor is set to the "Script" editor.

@RPicster tagging you because you mentioned that you're using search results in the scene view for navigation somehow. I personally can't make it work, when I click on those nothing changes in the 3D view for me, but if it works for you this change would likely break it, so pls comment

@AeioMuch AeioMuch force-pushed the search_results_button branch 3 times, most recently from acae627 to 3de179e Compare February 25, 2024 14:48
@AeioMuch AeioMuch changed the title [Editor] Don't add the Search Results bottom panel button until first search and add a close button to hide it. [Editor] Hide Search Results by default. Show it on first search and push it at the end. Add a close button to hide it back. Also switch to Script Editor if a searched line is clicked. Feb 25, 2024
@RPicster
Copy link
Contributor

RPicster commented Mar 7, 2024

I have also make it so that when you click on a line found, the main screen editor is set to the "Script" editor.

@RPicster tagging you because you mentioned that you're using search results in the scene view for navigation somehow. I personally can't make it work, when I click on those nothing changes in the 3D view for me, but if it works for you this change would likely break it, so pls comment

I think you can just forget what I said :D Watching my own behaviour I think it's not important.

@AThousandShips AThousandShips changed the title [Editor] Hide Search Results by default. Show it on first search and push it at the end. Add a close button to hide it back. Also switch to Script Editor if a searched line is clicked. [Editor] Hide Search Results by default and show it on first search. Mar 7, 2024
editor/plugins/script_editor_plugin.cpp Outdated Show resolved Hide resolved
…t the end. Add a close button to hide it back. Also switch to Script Editor if a searched line is clicked.
@AeioMuch AeioMuch force-pushed the search_results_button branch from 3de179e to 5cf6f3c Compare March 16, 2024 19:43
Copy link
Contributor

@Norrox Norrox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks alright to me

@akien-mga akien-mga modified the milestones: 4.x, 4.3 Mar 24, 2024
akien-mga added a commit that referenced this pull request Mar 25, 2024
[Editor] Hide Search Results by default and show it on first search.
@akien-mga akien-mga closed this pull request by merging all changes into godotengine:master in 0acfb38 Mar 25, 2024
@akien-mga
Copy link
Member

Thanks!

@AeioMuch AeioMuch deleted the search_results_button branch March 25, 2024 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't show the Search Results bottom panel button until you search for something
7 participants