Skip to content

Commit

Permalink
[Upgrade Assistant] Improve flyout information architecture (#111713)
Browse files Browse the repository at this point in the history
* Make sure longstrings inside flyout body are text-wrap

* Show resolved badge for reindex flyout and row

* Finish off rest of ES deprecation flyouts

* Refactor deprecation badge into its own component

* Add tests for kibana deprecations

* Add tests for es deprecations

* Also check that we have status=error before rendering error callout

* Check for non-complete states instead of just error

* Small refactor

* Default deprecation is not resolvable

* Add a bit more spacing between title and badge

* Address CR changes

* Use EuiSpacer instead of flexitems
  • Loading branch information
sabarasaba authored Sep 15, 2021
1 parent a49c70b commit 70f2677
Show file tree
Hide file tree
Showing 19 changed files with 171 additions and 143 deletions.
1 change: 0 additions & 1 deletion x-pack/plugins/translations/translations/ja-JP.json
Original file line number Diff line number Diff line change
Expand Up @@ -25845,7 +25845,6 @@
"xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.readonlyCallout.backgroundResumeDetail": "再インデックスはバックグラウンドで継続しますが、Kibana がシャットダウンまたは再起動した場合、このページに戻り再インデックスを再開させる必要があります。",
"xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.readonlyCallout.calloutTitle": "インデックスは再インデックス中にドキュメントを投入、更新、または削除できません",
"xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.readonlyCallout.cantStopDetail": "ドキュメントの更新を停止できない場合、または新しいクラスターに再インデックスする必要がある場合は、異なるアップグレード方法をお勧めします。",
"xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.reindexButton.doneLabel": "完了!",
"xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.reindexButton.reindexingLabel": "再インデックス中…",
"xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.reindexButton.resumeLabel": "再開",
"xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.reindexButton.runReindexLabel": "再インデックスを実行",
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/translations/translations/zh-CN.json
Original file line number Diff line number Diff line change
Expand Up @@ -26280,7 +26280,6 @@
"xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.readonlyCallout.backgroundResumeDetail": "重新索引将在后台继续,但如果 Kibana 关闭或重新启动,您将需要返回此页,才能恢复重新索引。",
"xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.readonlyCallout.calloutTitle": "在重新索引时,索引无法采集、更新或删除文档",
"xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.readonlyCallout.cantStopDetail": "如果您无法停止文档更新或需要重新索引到新的集群中,请考虑使用不同的升级策略。",
"xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.reindexButton.doneLabel": "已完成!",
"xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.reindexButton.reindexingLabel": "正在重新索引……",
"xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.reindexButton.resumeLabel": "恢复",
"xpack.upgradeAssistant.checkupTab.reindexing.flyout.checklistStep.reindexButton.runReindexLabel": "运行重新索引",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,14 @@ describe('Index settings deprecation flyout', () => {
});

it('removes deprecated index settings', async () => {
const { find, actions } = testBed;
const { find, actions, exists } = testBed;

httpRequestsMockHelpers.setUpdateIndexSettingsResponse({
acknowledged: true,
});

expect(exists('indexSettingsDetails.warningDeprecationBadge')).toBe(true);

await actions.indexSettingsDeprecationFlyout.clickDeleteSettingsButton();

const request = server.requests[server.requests.length - 1];
Expand All @@ -75,6 +77,8 @@ describe('Index settings deprecation flyout', () => {
expect(find('removeSettingsPrompt').length).toEqual(0);
// Verify the action button no longer displays
expect(find('indexSettingsDetails.deleteSettingsButton').length).toEqual(0);
// Verify the badge got marked as resolved
expect(exists('indexSettingsDetails.resolvedDeprecationBadge')).toBe(true);
});

it('handles failure', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ describe('Machine learning deprecation flyout', () => {

describe('upgrade snapshots', () => {
it('successfully upgrades snapshots', async () => {
const { find, actions } = testBed;
const { find, actions, exists } = testBed;

httpRequestsMockHelpers.setUpgradeMlSnapshotResponse({
nodeId: 'my_node',
Expand All @@ -64,6 +64,7 @@ describe('Machine learning deprecation flyout', () => {
status: 'complete',
});

expect(exists('mlSnapshotDetails.criticalDeprecationBadge')).toBe(true);
expect(find('mlSnapshotDetails.upgradeSnapshotButton').text()).toEqual('Upgrade');

await actions.mlDeprecationFlyout.clickUpgradeSnapshot();
Expand All @@ -86,10 +87,11 @@ describe('Machine learning deprecation flyout', () => {
// Reopen the flyout
await actions.table.clickDeprecationRowAt('mlSnapshot', 0);

// Flyout actions should be disabled if deprecation was resolved
expect(find('mlSnapshotDetails.upgradeSnapshotButton').props().disabled).toBe(true);
expect(find('mlSnapshotDetails.upgradeSnapshotButton').text()).toContain('Upgrade complete');
expect(find('mlSnapshotDetails.deleteSnapshotButton').props().disabled).toBe(true);
// Flyout actions should be hidden if deprecation was resolved
expect(exists('mlSnapshotDetails.upgradeSnapshotButton')).toBe(false);
expect(exists('mlSnapshotDetails.deleteSnapshotButton')).toBe(false);
// Badge should be updated in flyout title
expect(exists('mlSnapshotDetails.resolvedDeprecationBadge')).toBe(true);
});

it('handles upgrade failure', async () => {
Expand Down Expand Up @@ -133,12 +135,13 @@ describe('Machine learning deprecation flyout', () => {

describe('delete snapshots', () => {
it('successfully deletes snapshots', async () => {
const { find, actions } = testBed;
const { find, actions, exists } = testBed;

httpRequestsMockHelpers.setDeleteMlSnapshotResponse({
acknowledged: true,
});

expect(exists('mlSnapshotDetails.criticalDeprecationBadge')).toBe(true);
expect(find('mlSnapshotDetails.deleteSnapshotButton').text()).toEqual('Delete');

await actions.mlDeprecationFlyout.clickDeleteSnapshot();
Expand All @@ -158,10 +161,11 @@ describe('Machine learning deprecation flyout', () => {
// Reopen the flyout
await actions.table.clickDeprecationRowAt('mlSnapshot', 0);

// Flyout actions should be disabled if deprecation was resolved
expect(find('mlSnapshotDetails.deleteSnapshotButton').props().disabled).toBe(true);
expect(find('mlSnapshotDetails.deleteSnapshotButton').text()).toContain('Deletion complete');
expect(find('mlSnapshotDetails.upgradeSnapshotButton').props().disabled).toBe(true);
// Flyout actions should be hidden if deprecation was resolved
expect(exists('mlSnapshotDetails.upgradeSnapshotButton')).toBe(false);
expect(exists('mlSnapshotDetails.deleteSnapshotButton')).toBe(false);
// Badge should be updated in flyout title
expect(exists('mlSnapshotDetails.resolvedDeprecationBadge')).toBe(true);
});

it('handles delete failure', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ describe('Kibana deprecation details flyout', () => {
await actions.table.clickDeprecationAt(1);

expect(exists('kibanaDeprecationDetails')).toBe(true);
expect(exists('kibanaDeprecationDetails.warningDeprecationBadge')).toBe(true);
expect(find('kibanaDeprecationDetails.flyoutTitle').text()).toBe(manualDeprecation.title);
expect(find('manualStepsList').find('li').length).toEqual(
manualDeprecation.correctiveActions.manualSteps.length
Expand All @@ -66,6 +67,7 @@ describe('Kibana deprecation details flyout', () => {
await actions.table.clickDeprecationAt(0);

expect(exists('kibanaDeprecationDetails')).toBe(true);
expect(exists('kibanaDeprecationDetails.criticalDeprecationBadge')).toBe(true);
expect(find('kibanaDeprecationDetails.flyoutTitle').text()).toBe(
quickResolveDeprecation.title
);
Expand All @@ -87,8 +89,9 @@ describe('Kibana deprecation details flyout', () => {

// Resolve information should not display and Quick resolve button should be disabled
expect(exists('resolveSection')).toBe(false);
expect(find('resolveButton').props().disabled).toBe(true);
expect(find('resolveButton').text()).toContain('Resolved');
expect(exists('resolveButton')).toBe(false);
// Badge should be updated in flyout title
expect(exists('kibanaDeprecationDetails.resolvedDeprecationBadge')).toBe(true);
});

test('handles resolve failure', async () => {
Expand All @@ -103,6 +106,7 @@ describe('Kibana deprecation details flyout', () => {
await actions.table.clickDeprecationAt(0);

expect(exists('kibanaDeprecationDetails')).toBe(true);
expect(exists('kibanaDeprecationDetails.criticalDeprecationBadge')).toBe(true);
expect(find('kibanaDeprecationDetails.flyoutTitle').text()).toBe(
quickResolveDeprecation.title
);
Expand All @@ -123,6 +127,8 @@ describe('Kibana deprecation details flyout', () => {
expect(exists('quickResolveError')).toBe(true);
// Resolve information should display and Quick resolve button should be enabled
expect(exists('resolveSection')).toBe(true);
// Badge should remain the same
expect(exists('kibanaDeprecationDetails.criticalDeprecationBadge')).toBe(true);
expect(find('resolveButton').props().disabled).toBe(false);
expect(find('resolveButton').text()).toContain('Try again');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,11 @@ import {
EuiText,
EuiTextColor,
EuiLink,
EuiSpacer,
} from '@elastic/eui';

import { EnrichedDeprecationInfo } from '../../../../../../common/types';
import { DeprecationBadge } from '../../../shared';

export interface DefaultDeprecationFlyoutProps {
deprecation: EnrichedDeprecationInfo;
Expand Down Expand Up @@ -61,10 +63,10 @@ export const DefaultDeprecationFlyout = ({
return (
<>
<EuiFlyoutHeader hasBorder>
<DeprecationBadge isCritical={deprecation.isCritical} isResolved={false} />
<EuiSpacer size="s" />
<EuiTitle size="s" data-test-subj="flyoutTitle">
<h2 id="defaultDeprecationDetailsFlyoutTitle" className="eui-textBreakWord">
{message}
</h2>
<h2 id="defaultDeprecationDetailsFlyoutTitle">{message}</h2>
</EuiTitle>
{index && (
<EuiText data-test-subj="flyoutDescription">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export const DefaultTableRow: React.FunctionComponent<Props> = ({ rowFieldNames,
},
flyoutProps: {
onClose: closeFlyout,
className: 'eui-textBreakWord',
'data-test-subj': 'defaultDeprecationDetails',
'aria-labelledby': 'defaultDeprecationDetailsFlyoutTitle',
},
Expand All @@ -62,8 +63,8 @@ export const DefaultTableRow: React.FunctionComponent<Props> = ({ rowFieldNames,
>
<EsDeprecationsTableCells
fieldName={field}
openFlyout={() => setShowFlyout(true)}
deprecation={deprecation}
openFlyout={() => setShowFlyout(true)}
/>
</EuiTableRowCell>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
import { EnrichedDeprecationInfo, IndexSettingAction } from '../../../../../../common/types';
import type { ResponseError } from '../../../../lib/api';
import type { Status } from '../../../types';
import { DeprecationBadge } from '../../../shared';

export interface RemoveIndexSettingsFlyoutProps {
deprecation: EnrichedDeprecationInfo;
Expand Down Expand Up @@ -109,6 +110,11 @@ export const RemoveIndexSettingsFlyout = ({
return (
<>
<EuiFlyoutHeader hasBorder>
<DeprecationBadge
isCritical={deprecation.isCritical}
isResolved={statusType === 'complete'}
/>
<EuiSpacer size="s" />
<EuiTitle size="s" data-test-subj="flyoutTitle">
<h2 id="indexSettingsDetailsFlyoutTitle">{message}</h2>
</EuiTitle>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ export const IndexSettingsTableRow: React.FunctionComponent<Props> = ({
},
flyoutProps: {
onClose: closeFlyout,
className: 'eui-textBreakWord',
'data-test-subj': 'indexSettingsDetails',
'aria-labelledby': 'indexSettingsDetailsFlyoutTitle',
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
} from '@elastic/eui';

import { EnrichedDeprecationInfo } from '../../../../../../common/types';
import { DeprecationBadge } from '../../../shared';
import { MlSnapshotContext } from './context';
import { SnapshotState } from './use_snapshot_state';

Expand Down Expand Up @@ -51,12 +52,6 @@ const i18nTexts = {
defaultMessage: 'Retry upgrade',
}
),
upgradeResolvedButtonLabel: i18n.translate(
'xpack.upgradeAssistant.esDeprecations.mlSnapshots.flyout.deleteResolvupgradeResolvedButtonLabeledButtonLabel',
{
defaultMessage: 'Upgrade complete',
}
),
closeButtonLabel: i18n.translate(
'xpack.upgradeAssistant.esDeprecations.mlSnapshots.flyout.closeButtonLabel',
{
Expand All @@ -75,12 +70,6 @@ const i18nTexts = {
defaultMessage: 'Deleting…',
}
),
deleteResolvedButtonLabel: i18n.translate(
'xpack.upgradeAssistant.esDeprecations.mlSnapshots.flyout.deleteResolvedButtonLabel',
{
defaultMessage: 'Deletion complete',
}
),
retryDeleteButtonLabel: i18n.translate(
'xpack.upgradeAssistant.esDeprecations.mlSnapshots.flyout.retryDeleteButtonLabel',
{
Expand Down Expand Up @@ -119,8 +108,6 @@ const getDeleteButtonLabel = (snapshotState: SnapshotState) => {
switch (snapshotState.status) {
case 'in_progress':
return i18nTexts.deletingButtonLabel;
case 'complete':
return i18nTexts.deleteResolvedButtonLabel;
case 'idle':
default:
return i18nTexts.deleteButtonLabel;
Expand All @@ -138,8 +125,6 @@ const getUpgradeButtonLabel = (snapshotState: SnapshotState) => {
switch (snapshotState.status) {
case 'in_progress':
return i18nTexts.upgradingButtonLabel;
case 'complete':
return i18nTexts.upgradeResolvedButtonLabel;
case 'idle':
default:
return i18nTexts.upgradeButtonLabel;
Expand All @@ -155,6 +140,8 @@ export const FixSnapshotsFlyout = ({
upgradeSnapshot,
deleteSnapshot,
}: FixSnapshotsFlyoutProps) => {
const isResolved = snapshotState.status === 'complete';

const onUpgradeSnapshot = () => {
upgradeSnapshot();
closeFlyout();
Expand All @@ -168,12 +155,14 @@ export const FixSnapshotsFlyout = ({
return (
<>
<EuiFlyoutHeader hasBorder>
<DeprecationBadge isCritical={deprecation.isCritical} isResolved={isResolved} />
<EuiSpacer size="s" />
<EuiTitle size="s" data-test-subj="flyoutTitle">
<h2 id="mlSnapshotDetailsFlyoutTitle">{i18nTexts.flyoutTitle}</h2>
</EuiTitle>
</EuiFlyoutHeader>
<EuiFlyoutBody>
{snapshotState.error && (
{snapshotState.error && !isResolved && (
<>
<EuiCallOut
title={
Expand Down Expand Up @@ -207,40 +196,38 @@ export const FixSnapshotsFlyout = ({
</EuiButtonEmpty>
</EuiFlexItem>

<EuiFlexItem grow={false}>
<EuiFlexGroup>
<EuiFlexItem>
<EuiButtonEmpty
data-test-subj="deleteSnapshotButton"
color="danger"
onClick={onDeleteSnapshot}
isLoading={
snapshotState.action === 'delete' && snapshotState.status === 'in_progress'
}
disabled={
snapshotState.status === 'in_progress' || snapshotState.status === 'complete'
}
>
{getDeleteButtonLabel(snapshotState)}
</EuiButtonEmpty>
</EuiFlexItem>
<EuiFlexItem>
<EuiButton
fill
onClick={onUpgradeSnapshot}
isLoading={
snapshotState.action === 'upgrade' && snapshotState.status === 'in_progress'
}
disabled={
snapshotState.status === 'in_progress' || snapshotState.status === 'complete'
}
data-test-subj="upgradeSnapshotButton"
>
{getUpgradeButtonLabel(snapshotState)}
</EuiButton>
</EuiFlexItem>
</EuiFlexGroup>
</EuiFlexItem>
{!isResolved && (
<EuiFlexItem grow={false}>
<EuiFlexGroup>
<EuiFlexItem>
<EuiButtonEmpty
data-test-subj="deleteSnapshotButton"
color="danger"
onClick={onDeleteSnapshot}
isLoading={
snapshotState.action === 'delete' && snapshotState.status === 'in_progress'
}
isDisabled={snapshotState.status === 'in_progress'}
>
{getDeleteButtonLabel(snapshotState)}
</EuiButtonEmpty>
</EuiFlexItem>
<EuiFlexItem>
<EuiButton
fill
onClick={onUpgradeSnapshot}
isLoading={
snapshotState.action === 'upgrade' && snapshotState.status === 'in_progress'
}
isDisabled={snapshotState.status === 'in_progress'}
data-test-subj="upgradeSnapshotButton"
>
{getUpgradeButtonLabel(snapshotState)}
</EuiButton>
</EuiFlexItem>
</EuiFlexGroup>
</EuiFlexItem>
)}
</EuiFlexGroup>
</EuiFlyoutFooter>
</>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export const MlSnapshotsTableRowCells: React.FunctionComponent<TableRowProps> =
},
flyoutProps: {
onClose: closeFlyout,
className: 'eui-textBreakWord',
'data-test-subj': 'mlSnapshotDetails',
'aria-labelledby': 'mlSnapshotDetailsFlyoutTitle',
},
Expand Down
Loading

0 comments on commit 70f2677

Please sign in to comment.