-
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
[Lens] Handle missing fields gracefully #78173
[Lens] Handle missing fields gracefully #78173
Conversation
Pinging @elastic/kibana-app (Team:KibanaApp) |
This is a much more graceful way to handle this edge case, thank you. I have a couple suggestions for the UI implementation. In the config panel, don't replace the name of the configuration, keep showing the original (especially if they've modified the default) and just add the alert icon with a tooltip. In the dimension editor, don't change the text of the selected item. Use EuiFormRow's error text to explain the problem and add |
@elasticmachine merge upstream |
Hi @flash1293, I found two additional things that should be fixed:
|
|
||
const { columnId, uniqueLabel } = props; | ||
if (!selectedColumn) { | ||
return null; | ||
} | ||
|
||
if (currentFieldIsInvalid) { |
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.
- To change the styles to hover the whole element, you could add a class
anchorClassName="lnsLayerPanel__anchor"
to tooltip. I also adjusted the code to Caroline's suggestions about anchor, so if you could replace this if with:
if (currentFieldIsInvalid) {
return (
<EuiToolTip
content={
<p>
{i18n.translate('xpack.lens.configure.invalidConfigTooltip', {
defaultMessage: 'Invalid configuration.',
})}
<br />
{i18n.translate('xpack.lens.configure.invalidConfigTooltipClick', {
defaultMessage: 'Click for more details.',
})}
</p>
}
anchorClassName="lnsLayerPanel__anchor"
>
<EuiLink
color="danger"
id={columnId}
className="lnsLayerPanel__triggerLink"
onClick={props.onClick}
data-test-subj="lns-dimensionTrigger"
aria-label={i18n.translate('xpack.lens.configure.editConfig', {
defaultMessage: 'Edit configuration',
})}
title={i18n.translate('xpack.lens.configure.editConfig', {
defaultMessage: 'Edit configuration',
})}
>
<EuiFlexGroup gutterSize="s" alignItems="center" responsive={false}>
<EuiFlexItem grow={false}>
<EuiIcon size="s" type="alert" />
</EuiFlexItem>
<EuiFlexItem grow={true}>{selectedColumn.sourceField}</EuiFlexItem>
</EuiFlexGroup>
</EuiLink>
</EuiToolTip>
);
}
with:
…ana into lens/fix-missing-field-bug
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.
Screenshots look great to me 👍
@elasticmachine merge upstream |
1 similar comment
@elasticmachine merge upstream |
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.
Tested on Chrome/FF and reviewed. The design looks good but the BucketNestingEditor error is still here. Could you take a second look at my comment @flash1293 please?
…ana into lens/fix-missing-field-bug
@mbondyra Misunderstood your comment the first time, sorry. It should be fixed now. |
Re-tested on FF, all works as expected, thanks! :) |
@@ -21,6 +22,10 @@ function nestColumn(columnOrder: string[], outer: string, inner: string) { | |||
return result; | |||
} | |||
|
|||
function getFieldName(fieldMap: Record<string, IndexPatternField>, column: IndexPatternColumn) { | |||
return hasField(column) ? fieldMap[column.sourceField]?.displayName || column.sourceField : ''; |
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.
much better than what I suggested! 👏
💚 Build SucceededMetrics [docs]async chunks size
History
To update your PR or re-run it, just comment with: |
Fixes #73843
This PR addresses the case of fields in an index pattern disappearing or changing their properties, rendering existing visualization invalid.
It's caught in two places:
This gives the user to recover visualizations from this pretty common type of error instead of having to recreate the complete visualization from scratch.
Testing
myfield
(even if there is no data)