Fix several visual issues with find result highlighting: #6478
Conversation
peterflynn
commented
Jan 11, 2014
peterflynn
commented
- Reduce width of scrollbar tickmarks to match new, narrower custom-themed scrollbars from Custom scrollbars for Windows based on win 8 #6305 (should also look better on Mac, where the tickmarks were already too wide)
- Fix bug where showing/hiding a bottom panel didn't update the positioning of scrollbar tickmarks
- Results that span multiple tokens showed little orange dots in the text highlight between tokens (due to repeated corner rounding). There are still faint lines between tokens (due to box-shadow), but they're much fainter.
- Reduce width of scrollbar tickmarks to match new, narrower custom-themed scrollbars (should also look better on Mac, where the tickmarks were already too wide) - Fix bug where showing/hiding a bottom panel didn't update the positioning of scrollbar tickmarks - Results that span multiple tokens showed little orange dots in the text highlight between tokens (due to repeated corner rounding). There are still faint lines between tokens (due to box-shadow), but they're much fainter.
ghost
assigned
There was a problem hiding this comment.
This is a good width for my Win7 system, but not quite right for Mac 10.8. What do you think about having platform-specific widths?
Actually, should probably specify a non-platform-specific width for Win (which would also be picked up by Linux), and then override for Mac.
There was a problem hiding this comment.
@redmunds Hmm, I tested on my 10.8 MBP and this width looked good. Is it too wide on your machine? Not wide enough? Screenshot might be helpful (I can capture one on my machine later to compare, too).
There was a problem hiding this comment.
And fwiw this looked like a good size on my Ubuntu VM too.
There was a problem hiding this comment.
Before your change, tickmarks were 1(?) pixel wider than scrollbar. With your change, they fit inside scrollbar with 3(?) pixels to spare. Would be nice if they were exact scrollbar width like on Win.
There was a problem hiding this comment.
@redmunds Crud, I think the width depends on whether you have a mouse plugged in. With no mouse, it's far narrower. I think there is some fancy measuring logic in CM that tries to detect which case we're in. We could duplicate that here...
But for the time being (in the interest of getting the Win width fix merged this sprint), I wonder if we should just continue doing what master does -- err on one side or the other. I don't feel strongly which one; master assumes there is a mouse, this PR assumes trackpad. Which do you think is better for now?
There was a problem hiding this comment.
Doesn't matter to me -- I mentioned it in case it was an easy fix.
|
I'm not sure how to reproduce the 2nd and 3rd bullets in the description, so not sure about those. |
|
You can repro the 2nd bullet by getting some Find in Files results, then doing a regular find (causing tickmarks to appear), then closing the Find in Files panel. Or similar repro steps with the Markdown Preview bottom panel, etc. |
|
For the 3rd bullet just do a search that matches across more than one CM token. E.g. an identifier plus a punctuation/delimiter char. Example: Before fix With fix Note the little red dots around the "." in the selected result (before the fix). |
|
Here's a Mac screenshot: |
|
@peterflynn Done with review. Unless you have any further changes you want to make, I am ready to merge this. Let me know. |
|
@redmunds If you're ok merging it as-is that works for me. I will look into scrollbar measuring for next sprint as a follow-up improvement. |
|
Merging. |
Fix several visual issues with find result highlighting:
