Implement column sorting in the marker table#6083
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6083 +/- ##
==========================================
- Coverage 83.70% 83.68% -0.02%
==========================================
Files 337 337
Lines 35575 35708 +133
Branches 9878 10021 +143
==========================================
+ Hits 29778 29884 +106
- Misses 5369 5396 +27
Partials 428 428 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
It wasn't really obvious to me that Duration and Name columns support this sorting too, because the arrow is shown only on the Start column initially. I'd prefer having the arrow shown on any sortable table, which makes total sense to me and I think it's like that in many UXs. Otherwise, users also would need to click here and there to understand if a column is sortable or not (eg, inside the Call Tree). Wdyt @mstange ? |
|
That's interesting feedback. However, I prefer the current UX - reasons below. It also matches what macOS does, see e.g. the Activity Monitor app. I'm not a fan of the visual noise of having inactive arrows everywhere. Sortable tables are always sorted by some column, so there's always one visible arrow. One visible arrow implies that other columns in this table might be sortable too, and I think clicking a column to check if it's sortable is a small price to pay. Also, I believe sorting isn't something that needs to be suggested; if the user has a need to sort, they will go looking for a way to do it. Having ambient suggestions to try to sort by other columns can be counter-productive if it's not what they need. For example you could argue that sorting markers by name is almost always useless, so I wouldn't want to draw attention to this ability - but on the other hand there's no harm in letting the user do it in the rare case that they do find a use case where it's useful. |
mstange
force-pushed
the
push-nuyuywpytuqu
branch
2 times, most recently
from
b9085a6 to
8a0c670
Compare
When we make columns sortable, this will offer a bigger click target and mousedown feedback area. This commit does two things: - The distance between the column header text and the separator now becomes part of the header cell (using padding). - The width of the header cell now includes the padding (box-sizing: border-box), so it changes its meaning. I've updated the width values in the JS code so that the outcome looks the same as before. I adjusted the numbers based on these rules: - By default: minWidth and initialidth is increased by 10 - this matches the "includes padding" change because we now have 5px of padding on each side (10px total). - The first column increases by 5px less because it didn't have a 5px column separate distance in front of it in the past. - The columns with headerWidthAdjustment don't increase their width at all because they didn't contribute any separator distance in the past (because their preceding column has hideDividerAfter:true).
It's not used by any TreeView users yet. The next commit will use it for the marker table. The intention is to also use it for the function list.
The sort is also persisted in the URL.
mstange
force-pushed
the
push-nuyuywpytuqu
branch
from
8a0c670 to
d17fade
Compare
2 participants
Main | Deploy
This uses some code from #4265 but also reimplements a lot of it.