When popping a committed range, make it the current preview selection.

[fqueze]

Randell suggested this idea during the performance work week, and I liked it.

Deploy preview

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 88.45%. Comparing base (2f3b9cd) to head (737034d).

:exclamation: Current head 737034d differs from pull request most recent head c61e0e2. Consider uploading reports for the commit c61e0e2 to get more accurate results

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #4970 +/- ## ======================================= Coverage 88.45% 88.45% ======================================= Files 304 304 Lines 27311 27317 +6 Branches 7374 7377 +3 ======================================= + Hits 24158 24164 +6 Misses 2931 2931 Partials 222 222 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[julienw]

This looks good to me. The only small correctness issue is that I'd like { hasSelection: false } on a second click on the range; currently it's { hasSelection: true } with the same range as the current range. I admit that as a user the difference is very subtle but I'm wary of subtle consequences.

I'd also like second opinions from other folks, especially some heavy users.

[fqueze]

This looks good to me.

Thanks!

The only small correctness issue is that I'd like { hasSelection: false } on a second click on the range; currently it's { hasSelection: true } with the same range as the current range. I admit that as a user the difference is very subtle but I'm wary of subtle consequences.

Good catch! I had no idea one could clear the selection by clicking the current committed range. I now handled that case and added a test, because if I didn't know that use case, I might not be the only one so it could be easy to break in the future.

I'd also like second opinions from other folks, especially some heavy users.

How do you want to proceed? I thought we had consensus already after the discussion we had in Toronto.

[mstange]

Randell suggested this idea during the performance work week, and I liked it.

Pretty sure it was me who suggested it :)

... in response to Randell's complaints that it was too hard to look at the things that happened just before the current committed range.

So yes, I approve of this change, thanks for implementing it!

And thanks for the { hasSelection: false } fix, it was pretty annoying to have a useless zoom button floating in the middle of the timeline.