Show scroll track tickmarks for Find results #5191
Conversation
tickmark positioning on Mac. One item still TBD: what to do when window resizes.
* origin/master: (1504 commits) update HTMLSimpleDOM tests to verify errors fix indentation Ignore injected highlight nodes during live HTML text operations Updated by ALF automation. make default file tree sorting to be like Windows Update File shell callback comments. Minor comment tweaks. Updated by ALF automation. add a comma at the end of line 376. code review changes use beforeChange event to remove line class before line handles change fix position issues. fix error color per @larz0 use PopUpManager to close menus use a line class instead of the line number gutter to show error status remove _shouldClearErrors fix unit test errors. rename event. fix error offsets. fix issues with removeTempDirectory() in unit tests Updated by ALF automation. Updated by ALF automation. Update strings.js, merge new translates strings for latest version. Update README.md ... Conflicts: src/search/FindReplace.js
help with this. Cleanup older code: no hand cursor since tickmarks not clickable for now, move no-op check into ScrollTrackMarkers
|
Very cool! On the Mac, the tickmarks end up beneath the scrollbar when it's visible (and when you mouse over the scrollbar and the track appears, it becomes hard to see the tickmarks since the track is only slightly transparent). Not sure if there's any way to improve this since creating a div for the tickmarks above the scrollbar would block mouse input to it. I don't think it's a huge deal anyway since the tickmarks are more for seeing roughly where results are, not necessarily for clicking directly on, and they're visible while doing ordinary touch-scrolling. |
ghost
assigned 
|
The behavior on the mac is different depending on the scroll bar preferences. If you have auto-hide scrollbars, the tick marks end up below the scrollbar, as NJ mentioned. If the "Show scroll bars" setting is "always", the tick marks end up on top of the scrollbar. Again, not a huge deal (and actually kind of cool, since it makes the ticks more visible when scrollbars are persistent). |
| if (trackHt > 0) { | ||
| // Scrollbar visible: determine offset of track from top of scrollbar | ||
| if (brackets.platform === "win") { | ||
| trackOffset = 17; // Up arrow pushes down track |
There was a problem hiding this comment.
Is it safe to always assume the scroll up arrow is 17 pixels? Would it be safer to assume that the arrow is always square and use the width of the scrollbar instead?
There was a problem hiding this comment.
(if we do end up keeping this as a hard-coded value, it should be declared as a constant)
There was a problem hiding this comment.
Because of the extra padding that CM places on the scrollbar div, I'd have to do something like this:
var sbWidth = $sb[0].offsetWidth - 1; // CM adds an extra 1px space next to sb
trackOffset = sbWidth; // Up arrow pushes down track (assume arrow ht = sb width)
...which seems not necessarily less fragile than a magic number. So I'm sort of inclined to just hardcode it in a const, if you're ok with it.
|
Initial review complete. This is awesome! |
|
@larz0 -- can you check the color and size of the tick marks? They are pretty close to Chrome (and look good to me), but should get your blessing. |
|
Ah, if we have |
|
@gruehle
|
|
@gruehle looked at it the second time and I think it looks good. Disregard my previous screenshot. But width should be 15px for OS X. |
tweak comments, narrower tickmarks on Mac (per Larz), go back to window.* references in Async.
|
@gruehle Changes pushed! |
|
@peterflynn NJ and I were actually serious about pulling in underscore.js. Since that is a bit out of scope for a bonus feature like this, I went ahead and filed that as #5229. In the meantime, this looks great! Merging. |
Show scroll track tickmarks for Find results
|
@gruehle I think you meant underscore/lodash in your most recent comment too right? I was hoping to add debounce to improve typing performance elsewhere. |
|
Yes, I meant underscore. 😊 |
|
@peterflynn Did you ever look into why the tickmarks don't show up on top of the scrollbar in OS X? |
One more bonus feature to squeeze in this sprint, if anyone has cycles to review it :-)
This shows yellow tickmarks in the scrollbar track showing the position of each search result (similar to Chrome, WebStorm, etc). On Mac, where there's no scrollbar visible normally, the tickmarks just float on the right hand side (not sure if this is what those other apps do too).
The tickmarks are disabled for inline editors right now. Find is somewhat broken in inline editors anyway, so it's probably not a big loss.
The tickmark API isn't intended for reuse elsewhere yet, but I think eventually we should generalize it so it could also be used for linting errors/warnings, "diff marks", etc.