Skip to content

Multiple line support in Find Widget #79329

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

Merged
merged 16 commits into from
Aug 21, 2019
Merged

Multiple line support in Find Widget #79329

merged 16 commits into from
Aug 21, 2019

Conversation

rebornix
Copy link
Member

@rebornix rebornix commented Aug 16, 2019

Adding multiple line find and replace support in Find Widget. Our original implementation of the Find Widget takes quite a few assumptions of keybindings to use, Find Widget height and the query being single line. That means adding multiple line support may affect Find Widgets

  • Keybindings (we are introducing one more keybindings for inserting new lines in the input box)
  • Rendering (our current implementation of visibility change, fade in/out, etc takes the fixed height of Find Widget into account)
  • Accessibility should still work fine
  • We have quite a few features assuming that the find query is single line and we need to think about how they can work properly with multiple line support enabled.

Below verifications should pass before the PR gets merged

  • Keybindings
    • We use ctrl+enter to insert new line based on the feedback we received
    • shift+enter should be disabled on textarea as it's used to go to previous search result
    • Replace All uses ctrl+enter on Windows
  • Rendering
    • The height of the find widget should be updated when a new line is inserted
    • Find Widget fade in/out should work seamlessly when the height of the Find Widget is changed
    • Find Widget options/actions (toggleWord, regex, next/previous result, etc) should be positioned at the same place as before
    • When users resize the editor or the find widget by dragging the left side sash, the textarea width should be updated properly the same way as before
    • textarea should have a max height and scrolling inside it should not trigger scrolling in the editor
    • We show an empty view zone at the top of the editor to avoid find widget covering the editor context. The chances of overlapping increases and we should also improve the experience here.
  • Features
    • History navigation should work only when we are pressing arrow keys when the cursor is at the end or beginning of the textarea
    • Find and replace should honor editor line feed.
    • Seed search string from selection may now take multiple line selection into account
      • seed search string from selection only works when the selection is single line, when the setting is turned on.
    • Auto find in selection may now take multiple line selection into account
      • since we will not seed search string if the selection is multiple line, auto find in selection works the same as before
  • Verify Monaco Standalone

image

@rebornix rebornix changed the title update textarea width when find widget or editor resizes Multiple line support in Find Widget Aug 16, 2019
@rebornix rebornix self-assigned this Aug 16, 2019
@rebornix rebornix added this to the August 2019 milestone Aug 16, 2019
Copy link

@MJ-Mohith MJ-Mohith left a comment

Choose a reason for hiding this comment

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

This feature looks interesting. It will really help.
I have a concern when the number of lines increases in find widget. Is there any limits imposed on number of lines? What will be the case if the user inputs more than 80 lines, in such case how does the new feature impact the look of the find widget?

@kieferrm kieferrm mentioned this pull request Aug 19, 2019
37 tasks
@rebornix
Copy link
Member Author

@MJ-Mohith thanks for brining them up, good feedback.

Is there any limits imposed on number of lines

The max height of an input box will be limited to 6 of visible lines (wrapped lines), which is similar to Search Viewlet and SCM commit message box.

@MJ-Mohith
Copy link

@rebornix thats a good feature. I have a minor concern how the multi-line find and replace will work with following case:

Find ::
this
is
a
sample
text

Replace:
this
is
a

text

what will be the result, if we have an empty new line in between our text in a replace box

@rebornix rebornix merged commit 2067359 into master Aug 21, 2019
@rebornix rebornix deleted the rebornix/multiLineFind branch August 21, 2019 04:01
@rebornix
Copy link
Member Author

@MJ-Mohith the result would be

this
is
a

text

@MJ-Mohith
Copy link

@rebornix :: Thanks and Good work.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants