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

Multiple Selections Rendered with Search Box #5756

Closed
carlos-zamora opened this issue May 5, 2020 · 4 comments · Fixed by #5798
Closed

Multiple Selections Rendered with Search Box #5756

carlos-zamora opened this issue May 5, 2020 · 4 comments · Fixed by #5798
Assignees
@carlos-zamora
Labels
Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Terminal v1.1

Comments

@carlos-zamora
Copy link
Member

Environment

Windows Terminal version (if applicable): 0.11.1251.0

Any other software?

Steps to reproduce

  1. (Optional) cd to our repo's src\cascadia\CascadiaPackage
  2. ls -R (or just print a lot of output)
  3. Open search box (ctrl+shift+f)
  4. Search png a few times (at least until you force the viewport to scroll)
  5. Scroll up about half a page (current selection should be past halfway down your viewport)
  6. Search png again (just hit enter)

Expected behavior

Only one selection is visible at a time (because only one exists).

Actual behavior

bug image
@carlos-zamora carlos-zamora added Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Product-Terminal The new Windows Terminal. labels May 5, 2020
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels May 5, 2020
@DHowett-MSFT
Copy link
Contributor

Search probably doesn't lock the renderer when changing its state or trigger scrolling properly. Want to look into this for 1.0?

@carlos-zamora
Copy link
Member Author

I can. I think Michael did a recent PR adding a bunch of those locks so I'll take a look at that and try it.

@DHowett-MSFT DHowett-MSFT added Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. labels May 7, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label May 7, 2020
@DHowett-MSFT DHowett-MSFT added Needs-Tag-Fix Doesn't match tag requirements and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels May 7, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label May 7, 2020
@DHowett-MSFT DHowett-MSFT added this to the Terminal v1.1 milestone May 7, 2020
@DHowett-MSFT
Copy link
Contributor

It is so assigned.

@carlos-zamora carlos-zamora mentioned this issue May 7, 2020
Merged
1 task
@ghost ghost added the In-PR This issue has a related PR label May 7, 2020
@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label May 8, 2020
DHowett-MSFT pushed a commit that referenced this issue May 8, 2020
@carlos-zamora
Trigger scroll on scrolled selection (#5798)
75752ae 
## Summary of the Pull Request
We accidentally missed switching one `TriggerRedrawAll` to `TriggerScroll`. This does that.

## References
#5185 - applies logic from this PR

## PR Checklist
* [X] Closes #5756

## Validation Steps Performed
Followed bug repro steps.
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels May 8, 2020
DHowett-MSFT pushed a commit that referenced this issue May 8, 2020
@carlos-zamora @DHowett
Trigger scroll on scrolled selection (#5798)
8ece137 
## Summary of the Pull Request
We accidentally missed switching one `TriggerRedrawAll` to `TriggerScroll`. This does that.

## References
#5185 - applies logic from this PR

## PR Checklist
* [X] Closes #5756

## Validation Steps Performed
Followed bug repro steps.

(cherry picked from commit 75752ae)
@ghost
Copy link

ghost commented May 13, 2020

🎉This issue was addressed in #5798, which has now been successfully released as Windows Terminal Release Candidate v0.11.1333.0 (1.0rc2).:tada:

Handy links:

jelster pushed a commit to jelster/terminal that referenced this issue May 28, 2020
@carlos-zamora @jelster
Trigger scroll on scrolled selection (microsoft#5798)
f3612d9 
## Summary of the Pull Request
We accidentally missed switching one `TriggerRedrawAll` to `TriggerScroll`. This does that.

## References
microsoft#5185 - applies logic from this PR

## PR Checklist
* [X] Closes microsoft#5756

## Validation Steps Performed
Followed bug repro steps.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@carlos-zamora carlos-zamora

Labels
Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
Terminal v1.1
Development

Successfully merging a pull request may close this issue.

Trigger scroll on scrolled selection microsoft/terminal
2 participants
@carlos-zamora @DHowett-MSFT