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

Find/Replace overlay should have an X for closing #1991

Closed
Tracked by #2021
vogella opened this issue Jun 25, 2024 · 19 comments
Closed
Tracked by #2021

Find/Replace overlay should have an X for closing #1991

vogella opened this issue Jun 25, 2024 · 19 comments
Labels
enhancement New feature or request

Comments

@vogella
Copy link
Contributor

vogella commented Jun 25, 2024

I think we need an faster way to close the Find/Replace overlay. Current process seems to be:

1.) Move Cursor inside the Find Overlay
2.) Press ESC

VsCode has a button for this, which I think is very intuitive. I was searching for such a button before realizing that most likely I have to click into it and use the escape button to close it.

image

image

@Wittmaxi WDYT?

@vogella vogella added the enhancement New feature or request label Jun 25, 2024
@HannesWell
Copy link
Member

You can also close it by clicking Ctrl+F again (or twice if doesn't have the focus).

@Wittmaxi
Copy link

@vogella I have been trying to think for the last two days about how I could implement this in the Overlay.
I never thought this would be an issue, but I see why it is now that you point it out.

I will try to come up with a good solution and maybe we iterate a few times

@laeubi
Copy link
Contributor

laeubi commented Jun 27, 2024

(or twice if doesn't have the focus).

That's actually something I found feels "buggy" it should not matter if the control has focus or not, it should simply listen to ESC + CTRL+F on the main editor and handle accordingly.

@Wittmaxi
Copy link

@laeubi that is by design and really important to my use!
The idea is that I want to "quickly" get into the overlay while editing, no matter whether the Overlay is open already or not.

@HeikoKlare
Copy link
Contributor

it should simply listen to ESC + CTRL+F on the main editor and handle accordingly.

I agree that pressing Escape when being in the editor should, in my opinion, also close the overlay. Maybe that would even render an additional close button obsolete, as it makes it easy and fast enough to close the overlay?

@laeubi
Copy link
Contributor

laeubi commented Jun 28, 2024

that is by design and really important to my use!
The idea is that I want to "quickly" get into the overlay while editing, no matter whether the Overlay is open already or not.

I found it rather "surprising" and the first time I opend the new overlay that ways exactly my problem "how to close that thing?!?" because it now hangs over my code and I have no idea how to close it, e.g. I searched some text, found it, edit something in the document and then the search has no focus (what one can not really "see" in contrast to a dialog where the title-bar usually indicate this) and I needed a while to find out how to close. Performing a shortcut twice to close a dialog is something I never seen before.

Maybe that would even render an additional close button obsolete, as it makes it easy and fast enough to close the overlay?

A close button is the most obvious way and having one more icon don't seem to need more space also such button does not need any new functionality (just trigger exiting one) so I would find it useful anyways.

@BeckerWdf
Copy link
Contributor

BeckerWdf commented Jun 28, 2024

A close button is the most obvious way and having one more icon don't seem to need more space also such button does not need any new functionality (just trigger exiting one) so I would find it useful anyways.

I agree. A close button makes it obvious to new user how to close the overlay.

@vogella
Copy link
Contributor Author

vogella commented Jun 28, 2024

Sounds like we all agree that a close button would be nice. I think it is fine to have other means of closing it but such a button would make it obviously for users.

@HeikoKlare
Copy link
Contributor

Yes, fully agree.
I would summarize my resulting (and hopefully conforming to everyone's) expectation regarding closing the overlay as follows:

  1. The overlay should have an explicit close button (actual scope of the original issue and should be addressed first to have an obivious means of closing the overlay).
  2. The overlay should close on pressing Escape when either the overlay itself or its target editor has focus.
  3. The overlay should not close on pressing CTRL+F (in particular when the overlay itself has focus). On the contrary, I would expect CTRL+F to always behave the same (no matter whether pressed when the editor or the overlay has focus): open the overlay (if not yet open) and move focus to the search input field.

@HannesWell
Copy link
Member

HannesWell commented Jun 30, 2024

3. The overlay should not close on pressing CTRL+F (in particular when the overlay itself has focus).

Personally I find it useful and like it that I can use the same short-cut again to close the overlay.
All other statements are fine for, although I find using ESC intuitive, but I also understand that not everyone shares that view. :)

@laeubi
Copy link
Contributor

laeubi commented Jun 30, 2024

At least on Firefox STRG+F only focus the search again, also the "old" dialog does not close when pressing STRG+F again but when I press ESC ...

@HeikoKlare
Copy link
Contributor

Thanks for sharing the different opinions on pressing CTRL+F when the overlay has focus! Then we need to find some consensus on this.

I see the following reasons for having CTRL+F move focus to the search input field instead of closing the dialog:

  1. I am used to this behavior as it is implemented by several other tools (like VS code, Firefox, Chrome, somehow even the Windows Explorer). And in my opinion, if there is a common behavior / user experience shared across multiple applications, so that users are used to it, we should adopt it to make it easy for users to apply the functionality.
  2. It allows me to tab through the search options and quickly move focus back to the find input field by pressing CTRL+F without tabbing until I reach that field (or use the mouse to activate it).
  3. In general, I find it confusing to close something with the same key combination as I use for opening it. This is because if I am not completely sure where the current input focus is, I can still be totally sure that I can access a functionality with a specific key combination and that the result does not depend on where the current focus is.

Still my opinion is not that strong, so I appreciate further arguments for one option or the other.

@BeckerWdf
Copy link
Contributor

I agree with Heiko. Especially 1. is a very strong argument.

@Wittmaxi
Copy link

Wittmaxi commented Jul 1, 2024

The Dialog also receives focus when Ctrl+F is pressed instead of closing.
Find/Replace is something I do so often that I have the overlay open all the time. I very seldomly want or need to close the overlay, most of the time I just need to get into it to start typing.

I agree that it might be confusing that pressing Ctrl+F twice closes it again. I thought it would be sensible behavior but seeing that vscode and firefox don't close the interface on pressing Ctrl+F again, I am not so sure anymore. I'm open to removing that toggle behavior and relying on esc for closing the interface instead.

@HannesWell
Copy link
Member

I agree that pressing Ctrl+F should put the focus on the search text editor, that's all reasonable. But if the focus is already there I also find it convenient if the overlay would continue to close itself, just like it is doing it now. It's not a must have for me, but I find it nice to have. :)
As far as I understand it, in that case, pressing Ctrl+F does nothing otherwise, doesn't it? And I would argue that if one does not like to close the overlay, one should simply not press Ctrl+F again.

@HeikoKlare
Copy link
Contributor

As far as I understand it, in that case, pressing Ctrl+F does nothing otherwise, doesn't it?

Usual behavior (in other tools) when pressing CTRL+F is that the complete text within the input field is selected, so it is kind of a shortcut for a "Select All" operation within the input field.
I have no strong opinion whether we should adopt that behavior. Closing the overlay via CTRL+F if the find input field has focus is also kind of fine for me (although I personally slightly prefer to adopt common tool behavior).

@vogella
Copy link
Contributor Author

vogella commented Jul 2, 2024

@Wittmaxi could you add the closing X button to the find overlay? I think everyone agree that this would be useful (independent of the CTRL+F discussion)

@Wittmaxi
Copy link

Wittmaxi commented Jul 2, 2024

@vogella Heiko and I agreed to the following in ojr last Meeting

  1. we need an X in the overlay
  2. ESC should close the overlay even if the focus is on the editor

There are many "high priority" issues and many unmerged PRs, so I really need to prioritise. Breaking bugs come first, the search history came first. I will probably get around to building this feature next week, maybe during the eclipse gettogether.

#2021

For now, this is my roadmap showing my understanding of priorities.

If you need such a feature ASAP, the right place too hook the X-Button is probably as a new ToolItem in searchTools. You can use the other toolitems in the Overlay as reference, they use a Builder with a fluent library which makes addition really easy

HeikoKlare added a commit to HeikoKlare/eclipse.platform.ui that referenced this issue Jul 4, 2024
The find/replace overlay can currently only be closed via the shortcut
CTRL+F when the overlay has input focus. This change adds a close button
to the overlay for providing an easy and comprehensible way of closing
the overlay.

Contributes to
eclipse-platform#1991
HeikoKlare added a commit to HeikoKlare/eclipse.platform.ui that referenced this issue Jul 4, 2024
The find/replace overlay can currently only be closed via pressing
Escape or using the shortcut CTRL+F when the overlay has input focus.
This change adds a close button to the overlay for providing an easy and
comprehensible way of closing the overlay.

Contributes to
eclipse-platform#1991
HeikoKlare added a commit to HeikoKlare/eclipse.platform.ui that referenced this issue Jul 5, 2024
The find/replace overlay can currently only be closed via pressing
Escape or using the shortcut CTRL+F when the overlay has input focus.
This change adds a close button to the overlay for providing an easy and
comprehensible way of closing the overlay.

Contributes to
eclipse-platform#1991
vogella added a commit to vogellacompany/eclipse.platform.ui that referenced this issue Jul 5, 2024
The find/replace overlay introduces a new close icon similar to the
close icon used for the tab folder from
https://bugs.eclipse.org/bugs/show_bug.cgi?id=575395

This pull request updates the notification close icon which still looks
like the old tab icon before the change in
https://bugs.eclipse.org/bugs/show_bug.cgi?id=575395  and does not fit
anymore in the overall look and feel.

A follow-up PR could also update the active folder but find a new
"reddisch" color would require more testing on the different OS, while
with the regular icon we can build on the testing of
eclipse-platform#1991
vogella added a commit to vogellacompany/eclipse.platform.ui that referenced this issue Jul 5, 2024
The find/replace overlay introduces a new close icon similar to the
close icon used for the tab folder from
https://bugs.eclipse.org/bugs/show_bug.cgi?id=575395

This pull request updates the notification close icon which still looks
like the old tab icon before the change in
https://bugs.eclipse.org/bugs/show_bug.cgi?id=575395  and does not fit
anymore in the overall look and feel.

A follow-up PR could also update the active folder but find a new
"reddisch" color would require more testing on the different OS, while
with the regular icon we can build on the testing of
eclipse-platform#1991

Fixes: eclipse-platform#2035
HeikoKlare added a commit to HeikoKlare/eclipse.platform.ui that referenced this issue Jul 5, 2024
The find/replace overlay can currently only be closed via pressing
Escape or using the shortcut CTRL+F when the overlay has input focus.
This change adds a close button to the overlay for providing an easy and
comprehensible way of closing the overlay.

Contributes to
eclipse-platform#1991
vogella added a commit to vogellacompany/eclipse.platform.ui that referenced this issue Jul 5, 2024
The find/replace overlay introduces a new close icon similar to the
close icon used for the tab folder from
https://bugs.eclipse.org/bugs/show_bug.cgi?id=575395

This pull request updates the notification close icon which still looks
like the old tab icon before the change in
https://bugs.eclipse.org/bugs/show_bug.cgi?id=575395  and does not fit
anymore in the overall look and feel.

A follow-up PR could also update the active folder but find a new
"reddisch" color would require more testing on the different OS, while
with the regular icon we can build on the testing of
eclipse-platform#1991

Fixes: eclipse-platform#2035
HeikoKlare added a commit that referenced this issue Jul 5, 2024
The find/replace overlay can currently only be closed via pressing
Escape or using the shortcut CTRL+F when the overlay has input focus.
This change adds a close button to the overlay for providing an easy and
comprehensible way of closing the overlay.

Contributes to
#1991
@HeikoKlare
Copy link
Contributor

Since the original issue (add closing button) has been addressed via #2023 and the issue of closing via Escape has been moved to a dedicated issue (#2033), I close this.

vogella added a commit that referenced this issue Jul 5, 2024
The find/replace overlay introduces a new close icon similar to the
close icon used for the tab folder from
https://bugs.eclipse.org/bugs/show_bug.cgi?id=575395

This pull request updates the notification close icon which still looks
like the old tab icon before the change in
https://bugs.eclipse.org/bugs/show_bug.cgi?id=575395  and does not fit
anymore in the overall look and feel.

A follow-up PR could also update the active folder but find a new
"reddisch" color would require more testing on the different OS, while
with the regular icon we can build on the testing of
#1991

Fixes: #2035
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants