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.

Sign up for GitHub

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

Jump to bottom

[CLOSED] Issue 5237: Show "no results" for Find in Files #4940

Open
core-ai-bot opened this issue Aug 30, 2021 · 24 comments
Open

[CLOSED] Issue 5237: Show "no results" for Find in Files #4940

core-ai-bot opened this issue Aug 30, 2021 · 24 comments

Comments

@core-ai-bot
Copy link
Member

Issue by cosmosgenius
Saturday Sep 28, 2013 at 15:37 GMT
Originally opened as adobe/brackets#5371

This is for #5237.

Removed the promise from dialog and create a function doSearch() which is called from the keydown event handler.

Moved "dialog" object to module scope so that function _showSearchResults can directly handle it.

Adding a css class "no-results" to the input box when no results are present.

cosmosgenius included the following code: https://github.com/adobe/brackets/pull/5371/commits
@core-ai-bot
Copy link
Member Author

Comment by redmunds
Monday Sep 30, 2013 at 21:26 GMT

@cosmosgenius I made a first pass through your code and it looks pretty good!

My first question is actually for@larz0 -- I like how this code now uses the red border around search field to indicate no results, but is that enough? Seems like it should also display "no results".

@core-ai-bot
Copy link
Member Author

Comment by larz0
Monday Sep 30, 2013 at 21:34 GMT

@redmunds@cosmosgenius

Ok let's do this:

screen shot 2013-09-30 at 2 32 28 pm

When the user starts typing a new search switch it back to:

screen shot 2013-09-30 at 2 33 50 pm

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Monday Sep 30, 2013 at 21:46 GMT

Dialog allows typing during search phase. The text is not updated until search is complete so it's hard to see, but I can see edits briefly when dialog is being dismissed. Text field should be disabled when search is in progress.

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Monday Sep 30, 2013 at 21:53 GMT

Done with initial code review. Thanks for taking on this change. It's a bigger change than it first seemed, but this will make searching much better.

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Thursday Oct 03, 2013 at 15:20 GMT

I apologize for the late comments on this pull request, but I had a crazy idea about this.

For me, another annoying thing about Find in Files is that sometimes I search for something in the code, do whatever, think I'm done and close the results panel. Later (sometimes only a few milliseconds), I realize there's something else I wanted to do. Unfortunately, the data is now lost -- which is a shame since the results panel data is now updated as you edit your code. So, I'd like a way to re-open the FiF results to see the previous search.

What does that have to do with this pull request? I thought of this following workflow that may change what's been done so far in this PR:

  1. When you invoke Find in Files, the dialog is opened at the top, but the results panel is also opened.
  2. The results panel displays nothing if no search has been done yet, or previous results (which could be "no results") if a search has already been done.
  3. During a search, the dialog is disabled and the results panel displays "searching...".
  4. When search completes, results panel shows either a list of results (as it currently does) or a message such as "no results" if the search argument is not found. Seems like the dialog at top should always stay open. If so, keyboard shortcut could be used to toggle dialog/panel open/closed.

Maybe need a way to see results panel without dialog open at top?

@larz0@cosmosgenius@TomMalbran Any thoughts on this?

@core-ai-bot
Copy link
Member Author

Comment by larz0
Thursday Oct 03, 2013 at 16:10 GMT

@redmunds I remember@RaymondLim mentioned something about tabbed search panel with previous search results. Maybe we could wait for that? We can open the panel via a view search history icon in the modal bar or shortcut.

@core-ai-bot
Copy link
Member Author

Comment by larz0
Thursday Oct 03, 2013 at 16:12 GMT

The tab label could show just the searched text up to a number of characters and the full info could go below the tab.

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Thursday Oct 03, 2013 at 16:24 GMT

@larz0 I forgot about the tabbed interface (that I'm sure we'll have at some point). I think that solves the "want to see previous results" case because there less need to close the panel when they're sharing space. We might also want to preserve the previous results when the panel is closed, but I think that can wait until we have a tabbed panel.

In anticipation of a closed tabbed panel, should "no results" be displayed in the panel instead of the dialog? Also, how does that effect whether dialog remains open (or not)? I'm just trying to avoid doing work that may be made obsolete soon.

@core-ai-bot
Copy link
Member Author

Comment by larz0
Thursday Oct 03, 2013 at 16:35 GMT

If we want to show the panel on search we should move the search input on the panel so it feels like they're connected and stop the eyes from pogo-sticking.

@core-ai-bot
Copy link
Member Author

Comment by cosmosgenius
Thursday Oct 03, 2013 at 19:46 GMT

Seems like a lot of design changes. From the discussion it seems now all the responses to the user will be done on the search panel instead of the dialog, which would be in the disabled state when the search is going on.

And now rather than destroying the previous results we would store it and show it the next time FiF is opened.
I am confused on the tabbed interface part though.
Also on what@larz0 commented, moving the search input to the panel means complete removal of the dialog right?

@core-ai-bot
Copy link
Member Author

Comment by njx
Thursday Oct 03, 2013 at 19:55 GMT

Definitely seems like a bit of scope creep :) I haven't followed this PR, but could we accept this as is and add the other idea to the backlog?

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Thursday Oct 03, 2013 at 20:01 GMT

@cosmosgenius Sorry. I mentioned it because I thought it could actually simplify this, but I only made it more confusing. Let's continue with the original plan.

@core-ai-bot
Copy link
Member Author

Comment by cosmosgenius
Thursday Oct 03, 2013 at 20:07 GMT

i liked the part about keep the previous results and showing them when next time FiF is opened. I'll try implementing it and see how it goes. I'll update on this.

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Thursday Oct 03, 2013 at 20:12 GMT

@cosmosgenius This PR pretty close to being done. Let's finish this PR, and then submit that as a new PR.

@core-ai-bot
Copy link
Member Author

Comment by cosmosgenius
Thursday Oct 03, 2013 at 20:53 GMT

How would i cancel an already initiated search?? I guess it wont be required as currently text field is disabled when search is in progress, but would like to know on how we could achieve the same.

Other than this i am done.

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Thursday Oct 03, 2013 at 23:12 GMT

Currently, there is no mechanism for cancelling a search, but it would be easy to add. The _inScope() function could check a flag and return false for all remaining files.

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Friday Oct 04, 2013 at 00:28 GMT

This looks pretty good. We're finishing off Sprint 32, so I won't be able to merge this for a few days.

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Thursday Oct 10, 2013 at 00:52 GMT

I'm still seeing a JSLint error: 234 '_doSearch' was used before it was defined.

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Thursday Oct 10, 2013 at 00:55 GMT

This branch seems to have introduced a problem when you initiate a Find in Files with an empty search argument. The wait cursor and disabled modalbar never go away.

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Thursday Oct 10, 2013 at 01:00 GMT

Done with second code review. This is looking good. Just 1 bug and a little cleanup.

@core-ai-bot
Copy link
Member Author

Comment by cosmosgenius
Thursday Oct 10, 2013 at 18:30 GMT

I moved the query null check to _getQueryRegExp. Seem when the reg ex is invalid the same getting stuck situation is happening.

@core-ai-bot
Copy link
Member Author

Comment by cosmosgenius
Thursday Oct 10, 2013 at 18:35 GMT

If we press "enter key" for three to four times for a "noresult search" it doesn't respond to "enter or esc" keys anymore. Only after some interaction it works (like any other key or mouse click on input box). I couldn't figure out why it is happening. It seems something related to brackets-core.

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Thursday Oct 10, 2013 at 21:06 GMT

If we press "enter key" for three to four times for a "noresult search" it doesn't respond to "enter or esc" keys anymore.

Where are you doing that?

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Friday Oct 11, 2013 at 15:36 GMT

Added Trello Card to Icebox for side discussion: https://trello.com/c/KY4eJL7V/1038-find-in-files-preserve-previous-results

This was squashed into adobe/brackets#5477. Closing this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@core-ai-bot