-
Notifications
You must be signed in to change notification settings - Fork 11.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Gauge: Simplify gauge dimension panel options #76216
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, something like this was what I had in mind. It could even be a bit smarter, if where to use alignment factors like BarGauge, then it would know the longest gauge name and value and have a better sense of size constraint (but maybe not needed for a first iteration)
c86c496
to
722a522
Compare
@@ -29,8 +29,9 @@ composableKinds: PanelCfg: { | |||
common.SingleStatBaseOptions | |||
showThresholdLabels: bool | *false | |||
showThresholdMarkers: bool | *true | |||
minVizWidth: uint32 | *75 | |||
minVizHeight: uint32 | *75 | |||
sizing: common.BarGaugeSizing & (*"auto" | _) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can rename the BarGaugeSizing
to just GaugeSizing
or maybe even something even more generic like VizSizing
to make it more universal if desired - I don't feel too strongly on need to do this 🤷♂️
@torkelo I've updated this PR to have the same handling of gauge dimensions as recent bar gauge PR ( |
}, | ||
defaultValue: defaultOptions.sizing, | ||
showIf: (options: Options) => options.orientation !== VizOrientation.Auto, | ||
}) | ||
.addNumberInput({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @nmarrs Would you mind to replace number input with slider input?
.addNumberInput({ | |
.addSliderInput({ | |
path: 'minVizWidth', | |
name: 'Min width', | |
description: 'Minimum column width (vertical orientation)', | |
defaultValue: defaultOptions.minVizWidth, | |
settings: { | |
min: 0, | |
max: 300, | |
step: 1, | |
}, | |
showIf: (options: Options) => | |
options.sizing === BarGaugeSizing.Manual && options.orientation === VizOrientation.Vertical, | |
}) | |
.addSliderInput({ | |
path: 'minVizHeight', | |
name: 'Min height', | |
description: 'Minimum row height (horizontal orientation)', | |
defaultValue: defaultOptions.minVizHeight, | |
settings: { | |
min: 0, | |
max: 300, | |
step: 1, | |
}, | |
showIf: (options: Options) => | |
options.sizing === BarGaugeSizing.Manual && options.orientation === VizOrientation.Horizontal, | |
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please, adjust max value as needed 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
600 max value should be more than enough. In other cases, if e.g. user wants a bigger gauge - auto move should cover that. Let me know what you think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
checking it out locally now :) goal is to get this merged today (and associated docs PR) 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
600 seems reasonable enough 👍 updated PR with requested changes
@nmarrs btw, just noticed, shouldn't |
@Develer for the gauge panel |
minVizHeight: 75, | ||
minVizWidth: 75, | ||
minVizHeight: 200, | ||
minVizWidth: 200, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nmarrs this is a pretty big breaking change, radically changing the look of thousands of panels that used to show many gauges now just show a few and with a scrollbar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You bet it caused regression issues.. Not cool nmarrs.
Update the gauge panel with a similar approach taken in bar gauge panel (see this PR) for handling
auto
/manual
gauge sizing options. By defaultauto
size will use a reasonable hardcoded value (though in the future this could be made smarter) whilemanual
allows user to explicitly determine the size of their gauge.UX
Screen.Recording.2023-11-07.at.1.43.59.PM.mov
Demo dashboard.txt
Related to #71690 + #46819
For docs changes see #76240
Created a follow-up issue for adding an overflow notice inside of VizRepeater