-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Vis: Default editor] Euification of heatmap options tab #45766
[Vis: Default editor] Euification of heatmap options tab #45766
Conversation
…ns/heatmap # Conflicts: # src/legacy/core_plugins/kbn_vislib_vis_types/public/components/common/number_input.tsx # src/legacy/core_plugins/kbn_vislib_vis_types/public/components/options/gauge/ranges_panel.tsx # src/legacy/core_plugins/kbn_vislib_vis_types/public/components/options/index.ts # src/legacy/core_plugins/kbn_vislib_vis_types/public/types.ts # src/legacy/ui/public/vis/vis_types/vislib_vis_type.js
💔 Build Failed |
Pinging @elastic/kibana-app |
💔 Build Failed |
Hi @cchaos ! |
💔 Build Failed |
…ns/heatmap # Conflicts: # x-pack/plugins/translations/translations/ja-JP.json # x-pack/plugins/translations/translations/zh-CN.json
💔 Build Failed |
💔 Build Failed |
💚 Build Succeeded |
💚 Build Succeeded |
…ns/heatmap # Conflicts: # src/legacy/core_plugins/kbn_vislib_vis_types/public/gauge.js # src/legacy/core_plugins/kbn_vislib_vis_types/public/goal.js # src/legacy/core_plugins/kbn_vislib_vis_types/public/heatmap.js # src/legacy/core_plugins/kbn_vislib_vis_types/public/pie.js # src/legacy/ui/public/vis/vis_types/vislib_vis_type.js
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.
LGTM, checked locally, also tested with dark mode and pseudo-localization.
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.
src/legacy/core_plugins/kbn_vislib_vis_types/public/components/common/color_schema.tsx
Outdated
Show resolved
Hide resolved
src/legacy/core_plugins/kbn_vislib_vis_types/public/components/common/color_schema.tsx
Outdated
Show resolved
Hide resolved
…bana into EUIfication/options/heatmap
💚 Build Succeeded |
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.
Code LGTM, tested and didn't notice any bugs
defaultMessage: 'Percentage mode', | ||
})} | ||
paramName="percentageMode" | ||
value={stateParams.setColorRange ? false : stateParams.percentageMode} |
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.
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.
Because it can be confusing if the user sees "4 colors" in the config panel but that's really being overridden by the custom ranges they've established which may be 3 colors.
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.
@cchaos
The Percentage mode
switch (which Joe asked about) does not have any relation to the Number of colors
, it is a standalone param which is excluded from visualization config if Use custom ranges
is turned on! The same work for Number of colors
.
I suppose @flash1293 asked why we didn't do the same in the Labels
panel -> means to turn off
all of params (Rotate
and Overwrite automatic color
) if Show labels
is off.
Could you please confirm it is a right UX solution ? Thanks
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.
I think I'm still confused, can you show me some screenshots?
It's not. Thanks. |
…ns/heatmap # Conflicts: # src/legacy/core_plugins/kbn_vislib_vis_types/public/controls/heatmap_options.html
💚 Build Succeeded |
💔 Build Failed |
…ns/heatmap # Conflicts: # src/legacy/core_plugins/kbn_vislib_vis_types/public/components/options/metrics_axes/label_options.tsx
💚 Build Succeeded |
* EUIficate heatmap options * Move collections * Split labels in a panel * Remove angular templates * Remove unused translations * Fix functional tests * Fix validation * Fix UI details * Compress color picker * Improve types * Replace headings h2 with h3 * Add functional tests * Create setCustomRangeByIndex
…6886) * EUIficate heatmap options * Move collections * Split labels in a panel * Remove angular templates * Remove unused translations * Fix functional tests * Fix validation * Fix UI details * Compress color picker * Improve types * Replace headings h2 with h3 * Add functional tests * Create setCustomRangeByIndex
Summary
A part of #38273.
EUIfication of the
Options
tab in theHeatmap
visualization.Also fix #43666.
The PR also includes next changes:
<ColorRanges />
, which is used inGauge
,Goal
,Heatmap
vis. It will be also used inMetric
vis ---><ColorSchemaOptions />
, which is used inGauge
,Goal
,Heatmap
vis.Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.For maintainers