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 Widget UI enhancement #26498

Closed
3 tasks done
rebornix opened this issue May 11, 2017 · 22 comments
Closed
3 tasks done

Find Widget UI enhancement #26498

rebornix opened this issue May 11, 2017 · 22 comments
Assignees
Labels
editor-find Editor find operations under-discussion Issue is under discussion for relevance, priority, approach
Milestone

Comments

@rebornix
Copy link
Member

rebornix commented May 11, 2017

Ref Find Widget issues

  • Make it resizeable: #2657, #2220
  • Bug, Replace Box in Current File Truncated #5861
  • Cover search result: #5525

We have a bunch of issues talking about the size or position of Find Widget as it sometimes gets in the way. The root cause is that our single line input box can't handle long text sometimes however the find widget itself sometimes covers the result or text (of course as it's absolute positioned Overlay). There are three ways to mitigate this issue

  • Make it resizable. It consists of two dimensions. The first one is resizing the Find Widget horizontally so the find and replace input boxes can be large enough. The second one is resizing it vertically to make it a multiline textarea, this way we can support multiline find and replace.
  • Dock the Find Widget the same way as the query box in Settings page.
  • Change the position of Find Widget if it's covering the search result. Either take Chromium's solution (only reposition horizontally) or Daniel's proposal (the widget can move both vertically and horizontally).

Resize

The major problem is users find the input box is not wide enough to see all typed in query keywords. We can have two way to mitigate this issue.

1. Horizontal resize

Allow users to resize the find widget horizontally (the same way as Visual Studio). The drawback is editor width is your limit.

resize-horizontal

2. Use textarea when the content is long

Instead of a single line input box, we use textarea to support long text and multi line content (this requires other work though #15727)
textarea-find-widget

Re-Position

Right now the Find Widget is an absolute positioned overlay so it sometimes covers the search result. We have several different solutions to solve or mitigate this problem as below

1. Move Find Widget horizontally when it's covering the search result

moving-find-widget

This is the way how Chrome handles this issue but it's not a perfect solution as when the editor width is small or not large enough, it still covers the result.

2. Move Find Widget both horizontally and vertically

This is suggested by @Tyriar, moving the find widget downwards when the window is not wide enough, especially when we split the window

moving-find-widget-2

@rebornix rebornix self-assigned this May 11, 2017
@rebornix rebornix added editor-find Editor find operations under-discussion Issue is under discussion for relevance, priority, approach labels May 11, 2017
@rebornix rebornix added this to the May 2017 milestone May 11, 2017
@tjgupta
Copy link

tjgupta commented May 16, 2017

Option 5 - Have it docked at the bottom of the editor, similar to the way the "Problems", "Output", "Debug Console", and "Terminal" panels do. Then it doesn't need to be resized or moved. Pretty please. :)

@rebornix
Copy link
Member Author

@tjgupta I'm working on it :)

@kieferrm kieferrm mentioned this issue May 16, 2017
44 tasks
@isidorn
Copy link
Contributor

isidorn commented May 17, 2017

Hey @rebornix looking at this I believe you are facing some of the issue we are already facing with the debug toolbar, captured here #2513

Just some thoughts:

  • We should definetley not make this move both vertically and horizontally since we do not do this anywhere else in our UX. Introducing such a new concept does not allign well with our general workbench UX which is not very flexibile
  • If we decide to allow users to move it horizontally it should follow a similar pattern that the debug toolbar has (a drag handle)
  • As for the resize we should follow the similar pattern we are already using in the Git commit box and the Repl input editor (your option 2). But both the repl and the git input box use our editor, so you might conisder to use the editor there as well

My point is we should not introduce a new ux concept if we already have some solutions that are solving this.

fyi @jamiedawsonyoung @stevencl @bpasero

@bpasero
Copy link
Member

bpasero commented May 17, 2017

Just a random idea: what if we actually allow to dock the find widget to the top or bottom of the editor (fixed) and everything else in the editor is pushed away from that? I am not 100% convinced about a find widget that moves dynamically.

As an example of what I am talking about, see ST for example:

image

@Tyriar
Copy link
Member

Tyriar commented May 17, 2017

We had quite a bit of discussion in two of our meetings recently. Here are some of the thoughts captured there:

  • The compact find widget looks really nice imo.
  • Docking to top or bottom will solve all the usability issues but put us in the very large pool of editors that do it that way.
  • Docking to the top will shift the editor down when the find dialog is toggled, from what @rebornix showed it wasn't that jarring, but still something to think about.
  • Having the find widget at the top feels more natural to me as that's where my eyes naturally look.
  • An alternatively to the automatic moving is to add a grip icon like the debug toolbar and allow users to move it. This probably makes the most sense to completely forget about the custom location and revert to the default once the find widget is hidden.
  • Currently we can support many searched at once: Search viewlet across multiple files, find widget in editor group 1, find widget in editor group 2, etc.
  • There is also the option to merge the search viewlet and the find widget together as some other editor do. We would be losing functionality doing this, plus vscode would be using search outside of monaco but we still need to implement it within monaco.

@rebornix
Copy link
Member Author

@isidorn @bpasero thanks for your feedback. At the very beginning I don't want to dock the Find Widget as we already have Search Viewlet, so I came up with the four options I demonstrated above.

Isi:

We should definetley not make this move both vertically and horizontally
In our meetings I found that no one enjoys the floating Find Widget and yes we don't want to bring in new UX

If we decide to allow users to move it horizontally it should follow a similar pattern that the debug toolbar has (a drag handle)
The toolbar is inspiring, I'll come up with a mockup and share with you later.

As for the resize we should follow the similar pattern we are already using in the Git commit box
That's how I implemented it, they share the same component, with a flip option (textarea or input box)

Ben:

what if we actually allow to dock the find widget to the top or bottom of the editor (fixed)
I'll provide a prototype today and personally I start to love it.

Thanks @Tyriar for your summary, it represents most of our ideas and feedback. In addition to that

  • In the coming prototype, I'll allow users to choose where to dock the find widget, either Top or Bottom.
  • Dock Find Widget at Top will have the shifting problem @Tyriar mentioned, but the good part is users are already used to see the Find Widget at the top.
  • Dock Find Widget at Bottom can be friendly to new users from other editors and it won't lead to any shifting problem. But as it's near the panel, it's kind of verbose.

@rebornix
Copy link
Member Author

rebornix commented May 18, 2017

Dock Find Widget at the top of the editor

findwidgettop

Dock Find Widget at the bottom of the editor

findwidgetbottom

As we make the Find Widget full width, maybe we can make input box shorter and make the options richer, for example, use keywords Previous, Next instead of simple icons.

@bpasero
Copy link
Member

bpasero commented May 18, 2017

@rebornix I was not suggesting to make this the default location but having a grip-button to move the find widget around seems like a good thing to explore. I would like to selfhost on the find widget being in other locations to provide further feedback 👍

@isidorn
Copy link
Contributor

isidorn commented May 18, 2017

I would also like to self host on this to provide more feedback.
Looking at the pictures the docked find widget at the top / bottom feel too heavy for me.

The current find widget solution I like because it is similar to the chrome experience which every user on the planet is familar with

@rebornix
Copy link
Member Author

Thank you for all the detailed feedback. While implementing the docking solution, we found we can actually split this two issues completely. For the issue of covering search result, we can allow users to scroll beyond the first line by the height of Find Widget. It doesn't change Find Widget at all, you can only scroll beyond first line only when the Find Widget is visible so it won't cover anything.

findoverlay

@isidorn
Copy link
Contributor

isidorn commented May 19, 2017

Looks good. But when and how do you trigger this scroll beyond first line?

@rebornix
Copy link
Member Author

@isidorn when Find widget is opened, I add a view zone before the first line and its height is the same as the find widget. Besides, I adjust the scrollTop of the editor then you don't see any shifting when the view zone is added. Since the whitespace is already there, you can scroll to it.

@isidorn
Copy link
Contributor

isidorn commented May 19, 2017

@rebornix if I understand correctly that means that I will always be able to scroll beyond first line if the find widget is visible? I believe we should only allow that if we detect that find widget is covering some text.
Though it might not be a bad idea, I would like to try it out.

@rebornix
Copy link
Member Author

@isidorn yes that's my initial plan but I received +1 on being able to scroll beyond first line. Let's see users' feedback on it and decide if we want to narrow it down.

@Tyriar
Copy link
Member

Tyriar commented May 22, 2017

@rebornix if the scrolling beyond the top only happens when it needs to, I think this is the way to go personally. Potentially combined with the textbox expanding vertically when needed.

I'd like to try it out as well though 😄

@Tyriar
Copy link
Member

Tyriar commented May 23, 2017

@rebornix would it be possible to only allow scrolling out of the bounds if there is text under the find widget? Just got this which feels a bit awkward:

screen shot 2017-05-22 at 8 22 08 pm

@jamiedawsonyoung jamiedawsonyoung self-assigned this May 23, 2017
@rebornix
Copy link
Member Author

@jamiedawsonyoung I made two changes to the Find Widget for this work item.

  • You can resize the Find widget horizontally
  • You can scroll beyond the first line when the find widget is visible.

As we still have half a week before testing, let's see users' feedback on this two changes. Right now I can see @Tyriar is not happy with scrolling as above.

@Tyriar
Copy link
Member

Tyriar commented May 24, 2017

@rebornix I like it, it might be better if we only added the extra space when absolutely necessary though.

@isidorn
Copy link
Contributor

isidorn commented May 29, 2017

@rebornix I just played with the find wiget a bit, here's my feedback:

  • The horizontal resize is too subtle, I think it is just 1px wide. Let's try to make it 2, or 3px wide at least to make it more user friendly
  • I still believe we should only go before the first line if we know the find widget is covering some text

@isidorn
Copy link
Contributor

isidorn commented May 29, 2017

@rebornix I just played with the find wiget a bit, here's my feedback:

  • The horizontal resize is too subtle, I think it is just 1px wide. Let's try to make it 2, or 3px wide at least to make it more user friendly
  • I still believe we should only go before the first line if we know the find widget is covering some text

Good work overall!

@aeschli aeschli mentioned this issue May 29, 2017
@rebornix
Copy link
Member Author

rebornix commented Jun 1, 2017

Close this first first and I'll create new issue for further improvement in next iteration.

@rebornix rebornix closed this as completed Jun 1, 2017
@ArmorDarks
Copy link

Docking to top or bottom will solve all the usability issues but put us in the very large pool of editors that do it that way.

This is how all improvements should be pitched. Great argument. Hope UX designers will never see it, or the world will be doomed to eternal bikeshedding in sake of marketing.

Sorry for necroing, just couldn't resist.

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
editor-find Editor find operations under-discussion Issue is under discussion for relevance, priority, approach
Projects
None yet
Development

No branches or pull requests

7 participants