ui: add custom range option to counter track display menu#5222
ui: add custom range option to counter track display menu#5222Vikx001 wants to merge 1 commit intogoogle:mainfrom
Conversation
Vikx001
force-pushed
the
dev/vikashsharma/counter-track-custom-range
branch
2 times, most recently
from
911bfda to
9838368
Compare
stevegolton
left a comment
stevegolton
left a comment
There was a problem hiding this comment.
Looks like a good first attempt, but there are quite a few rough edges.
- The text on the y axis should indicate the scale of the counter rather than just reading 'custom'.
- We need to consider how to make it easier to enter very small or very large values.
- I seemed to encounter a bug while trying to enter a decimal value, though I have been seeing some bugs in the counter track's settings recently so that might be related to that.
| .pf-counter-track { | ||
| &__custom-range { | ||
| display: flex; | ||
| align-items: center; |
There was a problem hiding this comment.
Please use align-items: baseline with widgets - this will make sure to align the base line of the text in each field.
There was a problem hiding this comment.
Oh yeah ! fixed it now Switched to baseline so the text labels and input fields sit on the same line visually.
| gap: 4px; | ||
| padding: 4px 8px; | ||
| font-family: var(--pf-font-compact); | ||
| font-size: var(--pf-font-size-m); |
There was a problem hiding this comment.
I suspect font-family and font-size can be inherited from the parent menu and thus emitted here, but could be wrong.
There was a problem hiding this comment.
You're right, they do get inherited from the parent menu so I removed both.
Verified it still looks correct without them.
Adds a 'Custom Range' mode to the counter track's Display submenu, letting users specify exact min/max values for the Y axis. This is useful when comparing counter tracks across different trace files, where using the same fixed range makes values directly comparable. When 'Custom Range' is selected, two numeric text inputs appear inline in the submenu for min and max. Either field can be left blank to fall back to the data-derived bound for that side. Rounding is skipped for custom ranges to honour the user-specified values. A yMax <= yMin guard prevents division-by-zero in the renderer. Fixes: google#4441 Address review feedback: - Use type=text instead of type=number so the browser does not suppress the change event while typing a decimal (e.g. "1."), and to allow scientific notation (1e-9, 2.5e10) for very small or large values; parseFloat handles all valid JS numeric strings correctly. - Remove hardcoded yLabel="custom"; fall through to the existing toLabel(max-min) path so the y-axis shows a human-readable scale (e.g. "50K", "2G") consistent with all other display modes. - Use align-items:baseline in the custom-range row so label text and input text share the same baseline. - Remove font-family/font-size from SCSS as they are inherited from the parent menu.
Vikx001
force-pushed
the
dev/vikashsharma/counter-track-custom-range
branch
from
9838368 to
42aff4a
Compare
|
Thanks for the detailed review @stevegolton! Addressed all three points: 
1. Y-axis label 2. Very small/large values 3. Decimal entry bug |
2 participants
Fixes #4441
When comparing counter tracks between two runs, users need a fixed Y-axis range so both tracks are visually comparable. The existing modes (Zero-based, Min/Max, Log) all derive the range from the data, making cross-run comparison difficult.
Changes
Add a
customY display mode to the Display submenu with inline Min/Max text inputs. Either field can be left blank to fall back to the data-derived bound for that side. Rounding is skipped for custom values to honour the user-specified exact bounds. AyMax <= yMinguard prevents division-by-zero in the renderer.Files changed
base_counter_track.ts— newcustomyDisplay mode,yCustomMin/yCustomMaxfields inCounterOptions, updatedcomputeYRange()and both menu rendering paths (context menu + bulk settings descriptor)base_counter_track.scss(new) — styles for the inline custom range inputsperfetto.scss— imports the new stylesheet