[Vis: Default editor] EUIficate ranges param editor #38531
Conversation
💔 Build Failed |
sulemanof
added
Team:Visualizations
|
Pinging @elastic/kibana-app |
sulemanof
added
the
release_note:skip
💚 Build Succeeded |
|
I like the use of the prepend labels, however, I think it gets too repetitive and I lose track of the actual values. How about we remove the prepend labels and use a subdued
I also realize we probably need something for the IP ranges as well: |
💚 Build Succeeded |
|
@cchaos the PR updated with proposed changes 👌 |
💚 Build Succeeded |
| </EuiFlexItem> | ||
| <EuiFlexItem grow={false}> | ||
| <EuiButtonIcon | ||
| aria-label={i18n.translate('common.ui.aggTypes.ranges.removeRangeButtonAriaLabel', { |
There was a problem hiding this comment.
Can you duplicate this aria-label as the title attribute as well?
| } | ||
|
|
||
| function RangesParamEditor({ agg, value = [], setValue }: AggParamEditorProps<RangeValues[]>) { | ||
| const [ranges, setRanges] = useState(() => value.map(range => ({ ...range, id: generateId() }))); |
There was a problem hiding this comment.
Not sure you need to use a callback state here, couldn't you write: const [ranges, setRanges] = useState(value.map(...));
There was a problem hiding this comment.
I think the lazy init is used to prevent an id generation on every render.
There was a problem hiding this comment.
@flash1293 you are completely right
| import { AggParamEditorProps } from 'ui/vis/editors/default'; | ||
|
|
||
| const generateId = htmlIdGenerator(); | ||
| const isEmpty = (value: any) => value === undefined || value === null; |
There was a problem hiding this comment.
As long as you're using lodash helpers, why not use the equivalent helper to this?
There was a problem hiding this comment.
Not sure which lodash helper you are talking about, but if you mean isNil - it is only available from the version 4.0.0. (currently we are using 3.10.1)
💚 Build Succeeded |
* EUIficate ranges control * Remove unused translations * Fix screenreader issue * Add an initial case for safety * Remove labels, add icon between ranges * Add title for delete btn # Conflicts: # x-pack/plugins/translations/translations/zh-CN.json
* EUIficate ranges control * Remove unused translations * Fix screenreader issue * Add an initial case for safety * Remove labels, add icon between ranges * Add title for delete btn # Conflicts: # x-pack/plugins/translations/translations/zh-CN.json
Summary
Part of #30922.
EUIfication of
rangescontrol forRangeaggregation.The changes include:
-∞forFromand+∞forTo(fix Add warning for infinite values in a range aggregation #36558);The PR also includes updated UI for
IPv4 Rangeaggregation:Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.For maintainers