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

Terminal find matches style is inconsistent #147013

Closed
miguelsolorio opened this issue Apr 7, 2022 · 5 comments · Fixed by #149486
Closed

Terminal find matches style is inconsistent #147013

miguelsolorio opened this issue Apr 7, 2022 · 5 comments · Fixed by #149486
Assignees
@Tyriar
Labels
accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders on-testplan polish Cleanup and polish issue terminal-find Relating the terminal's find widget
Milestone
May 2022

Comments

@miguelsolorio
Copy link
Contributor

Search and the find widget use the background color to highlight matches:

CleanShot 2022-04-07 at 07 35 35@2x

CleanShot 2022-04-07 at 07 36 28@2x

And terminal omits the background color, making it inconsistent and unexpected:

CleanShot 2022-04-07 at 07 37 10@2x
@meganrogge
Copy link
Contributor

Yeah we did this because the find background makes the contrast really bad in the terminal. We plan to fix this by layering the decoration behind the text in the future.

@meganrogge meganrogge added the *as-designed Described behavior is as designed label Apr 8, 2022
@miguelsolorio
Copy link
Contributor Author

@meganrogge I understand the struggle of making it look good but we shouldn't start deviating from the pattern as that introduces inconsistencies and creates debt. If we plan to fix this in the future then we shouldn't close out this issue as well for tracking purposes.

@meganrogge meganrogge reopened this Apr 18, 2022
@meganrogge meganrogge added polish Cleanup and polish issue terminal-find Relating the terminal's find widget and removed *as-designed Described behavior is as designed labels Apr 18, 2022
@meganrogge meganrogge added this to the May 2022 milestone Apr 18, 2022
@miguelsolorio miguelsolorio mentioned this issue Apr 26, 2022
Closed
@kieferrm kieferrm mentioned this issue May 7, 2022
Closed
99 tasks
@Tyriar Tyriar mentioned this issue May 9, 2022
Closed
@Tyriar
Copy link
Member

Tyriar commented May 13, 2022

Some things we should call out in the release notes related to this:

  • Fixed issue which caused minimum contrast ratio to not work correctly sometimes
  • Aligned selection colors with the editor selection since we can now do so without worrying about contrast problems
  • Minimum contrast ratio will now "go the other way" when the contrast ratio cannot be met with just reducing or increasing luminance (Allow minimum contrast ratio to go in both directions if needed xtermjs/xterm.js#3720 should be done soon)
  • Selection performance improved when gpu acceleration is enabled (maybe not worth mentioning?)

Tyriar added a commit that referenced this issue May 13, 2022
@Tyriar
Update xterm and make term find styles consistent with editor
99e15df 
- A bunch of changes to xterm.js including bg/fg decoration overrides
- Tweak find colors to use background instead of border, align with the editor
- Change high contrast themes to also align, including selection bg
- Clear the find active result decoration on blur and when the selection changes

Fixes #147013
Fixes #145751
@Tyriar Tyriar mentioned this issue May 13, 2022
Merged
@Tyriar Tyriar added feature-request Request for new features or functionality accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues labels May 13, 2022
@Tyriar
Copy link
Member

Tyriar commented May 13, 2022

Created #149495 to make sure we remember to add a contrast improvements section 🙂

@vscodenpa vscodenpa added unreleased Patch has not yet been released in VS Code Insiders insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels May 13, 2022
@Tyriar Tyriar mentioned this issue May 27, 2022
Closed
3 tasks
@github-actions github-actions bot locked and limited conversation to collaborators Jun 27, 2022
lemanschik pushed a commit to code-oss-dev/code that referenced this issue Nov 25, 2022
@Tyriar
Update xterm and make term find styles consistent with editor
145bd95 
- A bunch of changes to xterm.js including bg/fg decoration overrides
- Tweak find colors to use background instead of border, align with the editor
- Change high contrast themes to also align, including selection bg
- Clear the find active result decoration on blur and when the selection changes

Fixes microsoft#147013
Fixes microsoft#145751
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@Tyriar Tyriar

Labels
accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders on-testplan polish Cleanup and polish issue terminal-find Relating the terminal's find widget
Projects
None yet
Milestone
May 2022
Development

Successfully merging a pull request may close this issue.

Update xterm and make term find styles consistent with editor microsoft/vscode
5 participants
@Tyriar @meganrogge @miguelsolorio @vscodenpa and others