Skip to content

Commit 4bae6fe

Browse files
[8.19] [Discover] Make icon only presentational (#217519) (#219696) (#220192)
# Backport This will backport the following commits from `main` to `8.19`: - [[Discover] Make icon only presentational (#217519) (#219696)](#219696) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Alejandro García Parrondo","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-05-06T06:32:05Z","message":"[Discover] Make icon only presentational (#217519) (#219696)\n\n## Summary\n\nRemoved the aria-label and onClick from the icon, this makes the icon\nonly presentational, the onClick still applies through the whole badge\nand the screen reader only detects it once.\n\n\n![image](https://github.com/user-attachments/assets/850c26f6-d6dc-45a6-ba7a-76b81a299e4c)\n\nSolves https://github.com/elastic/kibana/issues/217519\n\n### Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers should verify this PR satisfies this list as well.\n\n- [ ] Any text added follows [EUI's writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\nsentence case text and includes [i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n- [ ]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas added for features that require explanation or tutorials\n- [ ] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [ ] If a plugin configuration key changed, check if it needs to be\nallowlisted in the cloud and added to the [docker\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\n- [ ] This was checked for breaking HTTP API changes, and any breaking\nchanges have been approved by the breaking-change committee. The\n`release_note:breaking` label should be applied in these situations.\n- [ ] [Flaky Test\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\nused on any tests changed\n- [ ] The PR description includes the appropriate Release Notes section,\nand the correct `release_note:*` label is applied per the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\n\n### Identify risks\n\nDoes this PR introduce any risks? For example, consider risks like hard\nto test bugs, performance regression, potential of data loss.\n\nDescribe the risk, its severity, and mitigation for each identified\nrisk. Invite stakeholders and evaluate how to proceed before merging.\n\n- [ ] [See some risk\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)\n- [ ] ...","sha":"db6d75fd8210c631b967c32b846d641cee444971","branchLabelMapping":{"^v9.1.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Project:Accessibility","Team:DataDiscovery","backport:version","v9.1.0","v8.19.0"],"title":"[Discover] Make icon only presentational (#217519)","number":219696,"url":"https://github.com/elastic/kibana/pull/219696","mergeCommit":{"message":"[Discover] Make icon only presentational (#217519) (#219696)\n\n## Summary\n\nRemoved the aria-label and onClick from the icon, this makes the icon\nonly presentational, the onClick still applies through the whole badge\nand the screen reader only detects it once.\n\n\n![image](https://github.com/user-attachments/assets/850c26f6-d6dc-45a6-ba7a-76b81a299e4c)\n\nSolves https://github.com/elastic/kibana/issues/217519\n\n### Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers should verify this PR satisfies this list as well.\n\n- [ ] Any text added follows [EUI's writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\nsentence case text and includes [i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n- [ ]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas added for features that require explanation or tutorials\n- [ ] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [ ] If a plugin configuration key changed, check if it needs to be\nallowlisted in the cloud and added to the [docker\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\n- [ ] This was checked for breaking HTTP API changes, and any breaking\nchanges have been approved by the breaking-change committee. The\n`release_note:breaking` label should be applied in these situations.\n- [ ] [Flaky Test\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\nused on any tests changed\n- [ ] The PR description includes the appropriate Release Notes section,\nand the correct `release_note:*` label is applied per the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\n\n### Identify risks\n\nDoes this PR introduce any risks? For example, consider risks like hard\nto test bugs, performance regression, potential of data loss.\n\nDescribe the risk, its severity, and mitigation for each identified\nrisk. Invite stakeholders and evaluate how to proceed before merging.\n\n- [ ] [See some risk\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)\n- [ ] ...","sha":"db6d75fd8210c631b967c32b846d641cee444971"}},"sourceBranch":"main","suggestedTargetBranches":["8.19"],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/219696","number":219696,"mergeCommit":{"message":"[Discover] Make icon only presentational (#217519) (#219696)\n\n## Summary\n\nRemoved the aria-label and onClick from the icon, this makes the icon\nonly presentational, the onClick still applies through the whole badge\nand the screen reader only detects it once.\n\n\n![image](https://github.com/user-attachments/assets/850c26f6-d6dc-45a6-ba7a-76b81a299e4c)\n\nSolves https://github.com/elastic/kibana/issues/217519\n\n### Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers should verify this PR satisfies this list as well.\n\n- [ ] Any text added follows [EUI's writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing), uses\nsentence case text and includes [i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n- [ ]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas added for features that require explanation or tutorials\n- [ ] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n- [ ] If a plugin configuration key changed, check if it needs to be\nallowlisted in the cloud and added to the [docker\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\n- [ ] This was checked for breaking HTTP API changes, and any breaking\nchanges have been approved by the breaking-change committee. The\n`release_note:breaking` label should be applied in these situations.\n- [ ] [Flaky Test\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\nused on any tests changed\n- [ ] The PR description includes the appropriate Release Notes section,\nand the correct `release_note:*` label is applied per the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\n\n### Identify risks\n\nDoes this PR introduce any risks? For example, consider risks like hard\nto test bugs, performance regression, potential of data loss.\n\nDescribe the risk, its severity, and mitigation for each identified\nrisk. Invite stakeholders and evaluate how to proceed before merging.\n\n- [ ] [See some risk\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)\n- [ ] ...","sha":"db6d75fd8210c631b967c32b846d641cee444971"}},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Alejandro García Parrondo <[email protected]>
1 parent 84e552b commit 4bae6fe

File tree

2 files changed

+157
-85
lines changed

2 files changed

+157
-85
lines changed

src/platform/plugins/shared/discover/public/application/main/components/top_nav/solutions_view_badge.test.tsx

Lines changed: 157 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -10,108 +10,182 @@
1010
import React from 'react';
1111
import { of } from 'rxjs';
1212
import { SolutionsViewBadge } from './solutions_view_badge';
13-
import { render } from '@testing-library/react';
13+
import { render, screen, waitFor, within } from '@testing-library/react';
14+
import userEvent from '@testing-library/user-event';
1415
import { useDiscoverServices } from '../../../../hooks/use_discover_services';
1516
import { __IntlProvider as IntlProvider } from '@kbn/i18n-react';
1617

1718
jest.mock('../../../../hooks/use_discover_services');
19+
const useDiscoverServicesMock = jest.mocked(useDiscoverServices);
1820

19-
const useDiscoverServicesMock = useDiscoverServices as jest.Mock;
21+
const mockUseDiscoverServicesMock = ({
22+
getActiveSpaceReturn,
23+
isSolutionViewEnabled,
24+
canManageSpaces = true,
25+
}: {
26+
getActiveSpaceReturn: object | undefined;
27+
isSolutionViewEnabled: boolean;
28+
canManageSpaces?: boolean;
29+
}) => {
30+
useDiscoverServicesMock.mockReturnValue({
31+
spaces: {
32+
getActiveSpace$: jest.fn().mockReturnValue(of(getActiveSpaceReturn)),
33+
isSolutionViewEnabled,
34+
},
35+
docLinks: {
36+
ELASTIC_WEBSITE_URL: 'https://www.elastic.co',
37+
},
38+
addBasePath: (path: string) => path,
39+
capabilities: { spaces: { manage: canManageSpaces } },
40+
} as unknown as ReturnType<typeof useDiscoverServices>);
41+
};
42+
43+
const setup = () => {
44+
const user = userEvent.setup();
45+
46+
const { container } = render(
47+
<IntlProvider locale="en">
48+
<SolutionsViewBadge badgeText="Toggle popover" />
49+
</IntlProvider>
50+
);
51+
52+
return { container, user };
53+
};
2054

2155
describe('SolutionsViewBadge', () => {
22-
test('renders badge', async () => {
23-
useDiscoverServicesMock.mockReturnValue({
24-
spaces: {
25-
getActiveSpace$: jest.fn().mockReturnValue(
26-
of({
27-
id: 'default',
28-
solution: 'classic',
29-
})
30-
),
31-
isSolutionViewEnabled: true,
32-
},
33-
docLinks: { ELASTIC_WEBSITE_URL: 'https://www.elastic.co' },
34-
addBasePath: (path: string) => path,
35-
capabilities: { spaces: { manage: true } },
56+
describe('when the solution visibility feature is disabled', () => {
57+
it('does not render the badge', () => {
58+
// Given
59+
mockUseDiscoverServicesMock({
60+
getActiveSpaceReturn: {
61+
id: 'default',
62+
solution: 'classic',
63+
},
64+
isSolutionViewEnabled: false,
65+
});
66+
67+
// When
68+
const { container } = setup();
69+
70+
// Then
71+
expect(container).toBeEmptyDOMElement();
3672
});
73+
});
74+
75+
describe('when spaces is disabled (no active space available)', () => {
76+
it('does not render the badge', () => {
77+
// Given
78+
mockUseDiscoverServicesMock({
79+
getActiveSpaceReturn: undefined,
80+
isSolutionViewEnabled: true,
81+
});
3782

38-
const { container, findByTitle, findByRole } = render(
39-
<IntlProvider locale="en">
40-
<SolutionsViewBadge badgeText="Toggle popover" />
41-
</IntlProvider>
42-
);
43-
expect(container).not.toBeEmptyDOMElement();
44-
45-
const button = await findByTitle('Toggle popover');
46-
await button.click();
47-
const dialog = await findByRole('dialog');
48-
expect(dialog).not.toBeEmptyDOMElement();
83+
// When
84+
const { container } = setup();
85+
86+
// Then
87+
expect(container).toBeEmptyDOMElement();
88+
});
4989
});
5090

51-
test('does not render badge when active space is already configured to use a solution view other than "classic"', async () => {
52-
useDiscoverServicesMock.mockReturnValue({
53-
spaces: {
54-
getActiveSpace$: jest.fn().mockReturnValue(
55-
of({
91+
describe('when there is an active space', () => {
92+
describe('when the active space is configured to use a solution view other than "classic"', () => {
93+
it('does not render the badge', () => {
94+
// Given
95+
mockUseDiscoverServicesMock({
96+
getActiveSpaceReturn: {
5697
id: 'default',
5798
solution: 'oblt',
58-
})
59-
),
60-
isSolutionViewEnabled: true,
61-
},
62-
docLinks: { ELASTIC_WEBSITE_URL: 'https://www.elastic.co' },
63-
addBasePath: (path: string) => path,
64-
capabilities: { spaces: { manage: true } },
65-
});
99+
},
100+
isSolutionViewEnabled: true,
101+
});
66102

67-
const { container } = render(
68-
<IntlProvider locale="en">
69-
<SolutionsViewBadge badgeText="Toggle popover" />
70-
</IntlProvider>
71-
);
72-
expect(container).toBeEmptyDOMElement();
73-
});
103+
// When
104+
const { container } = setup();
74105

75-
test('does not render badge when spaces is disabled (no active space available)', async () => {
76-
useDiscoverServicesMock.mockReturnValue({
77-
spaces: {
78-
getActiveSpace$: jest.fn().mockReturnValue(of(undefined)),
79-
isSolutionViewEnabled: true,
80-
},
81-
docLinks: { ELASTIC_WEBSITE_URL: 'https://www.elastic.co' },
82-
addBasePath: (path: string) => path,
83-
capabilities: { spaces: { manage: true } },
106+
// Then
107+
expect(container).toBeEmptyDOMElement();
108+
});
84109
});
85110

86-
const { container } = render(
87-
<IntlProvider locale="en">
88-
<SolutionsViewBadge badgeText="Toggle popover" />
89-
</IntlProvider>
90-
);
91-
expect(container).toBeEmptyDOMElement();
92-
});
93-
94-
test('does not render badge when solution visibility feature is disabled', async () => {
95-
useDiscoverServicesMock.mockReturnValue({
96-
spaces: {
97-
getActiveSpace$: jest.fn().mockReturnValue(
98-
of({
111+
describe('when the active space is configured to use the classic solution view', () => {
112+
it('renders the badge', () => {
113+
// Given
114+
mockUseDiscoverServicesMock({
115+
getActiveSpaceReturn: {
99116
id: 'default',
100117
solution: 'classic',
101-
})
102-
),
103-
isSolutionViewEnabled: false,
104-
},
105-
docLinks: { ELASTIC_WEBSITE_URL: 'https://www.elastic.co' },
106-
addBasePath: (path: string) => path,
107-
capabilities: { spaces: { manage: true } },
108-
});
118+
},
119+
isSolutionViewEnabled: true,
120+
canManageSpaces: false,
121+
});
122+
123+
// When
124+
setup();
125+
126+
// Then
127+
expect(screen.getByText('Toggle popover')).toBeVisible();
128+
});
109129

110-
const { container } = render(
111-
<IntlProvider locale="en">
112-
<SolutionsViewBadge badgeText="Toggle popover" />
113-
</IntlProvider>
114-
);
115-
expect(container).toBeEmptyDOMElement();
130+
describe('when the user clicks the badge', () => {
131+
describe('and the user has manage spaces capability', () => {
132+
it('opens the popover', async () => {
133+
// Given
134+
mockUseDiscoverServicesMock({
135+
getActiveSpaceReturn: {
136+
id: 'default',
137+
solution: 'classic',
138+
},
139+
isSolutionViewEnabled: true,
140+
canManageSpaces: true,
141+
});
142+
143+
// When
144+
const { user } = setup();
145+
146+
// Then
147+
await user.click(screen.getByTitle('Toggle popover'));
148+
149+
const dialog = screen.getByRole('dialog');
150+
await waitFor(() =>
151+
expect(
152+
within(dialog).getByText(
153+
`We improved Discover so your view adapts to what you're exploring. Choose Observability or Security as your “solution view” in your space settings.`
154+
)
155+
).toBeVisible()
156+
);
157+
});
158+
});
159+
160+
describe('and the user does not have manage spaces capability', () => {
161+
it('opens the popover', async () => {
162+
// Given
163+
mockUseDiscoverServicesMock({
164+
getActiveSpaceReturn: {
165+
id: 'default',
166+
solution: 'classic',
167+
},
168+
isSolutionViewEnabled: true,
169+
canManageSpaces: false,
170+
});
171+
172+
// When
173+
const { user } = setup();
174+
175+
// Then
176+
await user.click(screen.getByTitle('Toggle popover'));
177+
178+
const dialog = screen.getByRole('dialog');
179+
await waitFor(() =>
180+
expect(
181+
within(dialog).getByText(
182+
`We enhanced Discover to adapt seamlessly to what you're exploring. Select Observability or Security as the “solution view” — ask your admin to set it in the space settings.`
183+
)
184+
).toBeVisible()
185+
);
186+
});
187+
});
188+
});
189+
});
116190
});
117191
});

src/platform/plugins/shared/discover/public/application/main/components/top_nav/solutions_view_badge.tsx

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,6 @@ export const SolutionsViewBadge: FunctionComponent<{ badgeText: string }> = ({ b
5353
iconSide="right"
5454
onClick={() => setIsPopoverOpen((value) => !value)}
5555
onClickAriaLabel={onClickAriaLabel}
56-
iconOnClick={() => setIsPopoverOpen((value) => !value)}
57-
iconOnClickAriaLabel={onClickAriaLabel}
5856
>
5957
{badgeText}
6058
</EuiBadge>

0 commit comments

Comments
 (0)