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

Umbrella Issue: find/replace overlay issues #2021

Open
49 of 67 tasks
Wittmaxi opened this issue Jul 1, 2024 · 36 comments
Open
49 of 67 tasks

Umbrella Issue: find/replace overlay issues #2021

Wittmaxi opened this issue Jul 1, 2024 · 36 comments
Labels
enhancement New feature or request

Comments

@Wittmaxi
Copy link
Contributor

Wittmaxi commented Jul 1, 2024

This Issue documents my views on priority for the issues of the find/replace overlay.
High issues are issues which are blocking a possible release

high

medium

low

unclear

@Wittmaxi Wittmaxi added the enhancement New feature or request label Jul 1, 2024
@Wittmaxi
Copy link
Contributor Author

Wittmaxi commented Jul 1, 2024

@HeikoKlare did I miss anything we discussed in our meeting?

@HeikoKlare
Copy link
Contributor

Priorities look sound. Just the following issues were missing (I have added them to the initial post with the mentioned priorities):

@arsenalzp
Copy link
Contributor

arsenalzp commented Jul 3, 2024

Hello,
Any of those issues are for newcomers (good-first-issue)?

@Wittmaxi
Copy link
Contributor Author

Wittmaxi commented Jul 3, 2024

@arsenalzp Hi, if you are looking to get into them I can give you some pointers, just ask in the corresponding issues :)

These should be (relatively) straight-forward to do:
#1991
#2012

This one might is a bit harder, but maybe you can help me by creating a prototype for how such a resize-mechanism could look like?
#1996

Some issues are already actively being fixed, others are more involved. I can of course help you get into them too, just ask 😀

Please add me to the reviewers of your PRs so that I can roughly see what is happening in the Overlay!

@arsenalzp
Copy link
Contributor

@arsenalzp Hi, if you are looking to get into them I can give you some pointers, just ask in the corresponding issues :)

These should be (relatively) straight-forward to do: #1991 #2012

This one might is a bit harder, but maybe you can help me by creating a prototype for how such a resize-mechanism could look like? #1996

Some issues are already actively being fixed, others are more involved. I can of course help you get into them too, just ask 😀

Please add me to the reviewers of your PRs so that I can roughly see what is happening in the Overlay!

Thank you for detail explanation!
Let me explore the project and its architecture a little bit :)

@HeikoKlare
Copy link
Contributor

I have just added #2033 (high prio) and #2034 (low prio) to the list.

@lukeu
Copy link

lukeu commented Jul 5, 2024

Early adopter feedback - maybe consider adding #1813 to the list?

I almost logged an issue but realised the prior issue existed. I'd seen it occasionally with ALT+click to make multiple cursors (which I don't do often). I think the Find overlay will make this much easier to encounter, however.

Say I need to do a multple replace. Often I'll copy the "new" text to the clipboard, select the old text and then...

  • Maybe I'll even just use ["Find Again", "Paste"] keys a few times (very fast, no find UI needed), or
  • If there's more than a few hits, I'll...
    • Press Ctrl+F (the "old" text is entered into the Find Overlay - nice)
    • Press Ctrl+Enter (a multi-selection is made - nice)
    • Whack ESC, Paste

The last step just seems more quick & natural to me than [expand the overlay, paste it there, replace, close it]

The problem is: if Find found 100 hits - this bug inserts 10,000 copies (!)

@HeikoKlare
Copy link
Contributor

Thank you for your early adopter feedback, @lukeu! It is great to see you testing new functionality and then also giving feedback on it. That's very valuable!

I agree that the reported bug is quite annoying. Personally, I did not use the described workflow so far but rely on the "replace all" functionality of the find/replace dialog/overlay, but I see how beneficial the workflow can be (and when you can use it, it should of course work as expected anyway).
Since it's not a bug introduced by the new UI, I also do not see an urgent need of having it fixed in the context of the find/replace overlay and would consider it with lower priority than other issues that might reduce acceptance and/or adoption of the new UI. Still, as you mentioned that the issue becomes more "obvious" with the new UI (#1813 (comment)), I would propose to simply put it on this issue's list (even though it might be priority "low" in the context of this issue).

@Wittmaxi
Copy link
Contributor Author

Wittmaxi commented Jul 8, 2024

I am adding #2053
with high priority

@HeikoKlare
Copy link
Contributor

Also add #2046 (with high prio)?

@Wittmaxi
Copy link
Contributor Author

Wittmaxi commented Jul 8, 2024

@HeikoKlare Yes! Done!

@Wittmaxi
Copy link
Contributor Author

Wittmaxi commented Jul 8, 2024

Added #2055 with medium priority
Added #2054 with high priority

@Wittmaxi
Copy link
Contributor Author

Wittmaxi commented Jul 8, 2024

Added #2059 with high priority
@HeikoKlare is this related to your recent changes to the positioning of the overlay?

@HeikoKlare
Copy link
Contributor

@HeikoKlare
Copy link
Contributor

Added with medium priority (feel free to change if you see a different prio fitting):

@Wittmaxi
Copy link
Contributor Author

Medium Prio:
#2099

Low Prio:
#2100

@Wittmaxi
Copy link
Contributor Author

Low prio: #2105

@HeikoKlare
Copy link
Contributor

@HeikoKlare
Copy link
Contributor

@nettozahler
Copy link

nettozahler commented Aug 16, 2024

Thank you for all the work on this find/replace overlay. Using it I have found something which looks like a bug to me. In NON-regex (standard) mode one can simply add a phrase in the search and replace fields and change one occurrence after the other by pressing ENTER repeatedly. But this does not work for me in regex mode. Here hitting ENTER has no effect and I must use the "arrow down" button to jump to the next occurrence each time. I have not found an existing bug report for this issue yet. If you confirm this being a bug I can open a new bug report. But maybe I am missing something?

@HeikoKlare
Copy link
Contributor

Thank you for reporting the issue, @nettozahler! We were not aware of that bug yet (and I agree that it is a bug), so we do not have a GitHub issue for it yet. It would be great if you can open a new issue for that. It should be easy to fix and would be great to still have it in the upcoming release, so we will have a look as soon as possible.

@nettozahler
Copy link

Here is the fresh bug report: #2203 (comment). I think I cannot add it to this "umbrella" myself?

@akurtakov
Copy link
Member

There seems to be huge issue with this feature under Linux/wayland eclipse-platform/eclipse.platform.swt#1447

@HeikoKlare
Copy link
Contributor

There seems to be huge issue with this feature under Linux/wayland eclipse-platform/eclipse.platform.swt#1447

To me, it's actually not appears to be a bug of this feature but a bug of the Shell implementation when used on Wayland, see eclipse-platform/eclipse.platform.swt#1447 (comment)

@akurtakov
Copy link
Member

There seems to be huge issue with this feature under Linux/wayland eclipse-platform/eclipse.platform.swt#1447

To me, it's actually not appears to be a bug of this feature but a bug of the Shell implementation when used on Wayland, see eclipse-platform/eclipse.platform.swt#1447 (comment)

That's exactly why I reported it to swt but still it's a major regression in the overall UX.

@HeikoKlare
Copy link
Contributor

Yes, it's definitely a regression. At least, this is something that you can configure via preferences, so you are not stuck with the bad UX. But still it's a bad experience to see this as default behavior on a fresh installation. And, of course, you need to know (1) that you can configure it and (2) where you have to configure it, since the popup easing this process for you does not appear either. I do not see a way of automatically disabling the feature (for Wayland), except for code-based changes.

If I am not mistaken, the welcome page is also static, local content, isn't it? Otherwise, if that information was dynamically loaded from a remote URL and could thus be changed after deployment of the application, it could be a means to inform users about the issue (and the simple solution for it).

@deepika-u
Copy link
Contributor

A query: Do we have an option to clear the history and have a fresh set of history i want to build/accumulate from now on?

@HeikoKlare
Copy link
Contributor

A query: Do we have an option to clear the history and have a fresh set of history i want to build/accumulate from now on?

If I am not mistaken, there is no such option yet. It might be possible to clear the dialog settings, in which the history is stored, manually, but not in a proper way via the UI. I don't see a necessity for it since old history entries will simply be removed upon addition of new entries (and the existing dialog also had not option for it), but if you find that a useful function, feel free to create an issue from it.

@stephan-herrmann
Copy link
Contributor

To me #2473 feels like a significant regression.

@HeikoKlare
Copy link
Contributor

To me #2473 feels like a significant regression.

I have added #2473 to the list of issues, but my current assessment regarding "significance" of the issue is different. It only affects a very specific workflow, so that if you are used to that workflow, it is probably annoying, but I am unsure whether the workflow is widely used.

@tobiasmelcher
Copy link
Contributor

#2509 might be added to this list

@HeikoKlare
Copy link
Contributor

#2509 might be added to this list

Thank you for reporting this. I've added it to the list with medium priority.
I also ranked #2161 up to high priority, as I consider this the most important of the currently open issues, in particular higher than the other medium priority ones.

@HannesWell
Copy link
Member

#2509 might be added to this list

Thank you for reporting this. I've added it to the list with medium priority.

Can you please reevaluate the priority? At least I personally find it very annoying when running into the described situation.

@HeikoKlare
Copy link
Contributor

Can you please reevaluate the priority? At least I personally find it very annoying when running into the described situation.

It's fine for me to increase the priority. In particular, since the priorities should be driven by demand of the community and initial priorities (assigned by an individual) may be driven by an the individual's assessment with incomplete assumptions about the usage of features. So feel free to simply adapt the priorities here. Just keep in mind that they should really reflect the according relevance, as people who want to take over issues and contribute fixes for them should be properly guided in selecting a reasonable issue by the list of priorities.

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

10 participants