Skip to content

Commit 1b41310

Browse files
[Dataset Quality] Fix link to discover in error type being truncated out for smaller screens (#240400)
Closes #239139 ## Summary Fixes a couple of issues with the link to discover in the error type column of data quality issues. Namely: ### Anchor tag being truncated out The error type component was previously made of a composition of EuiBadge and EuiLink. This meant that the anchor tag was scoped only on the external link icon inside the badge, which depending on the length of the error description as well as screen size, could have the icon truncated out making users unable to make use of the direct link to discover. This PR updates the component to use a single EuiButton instead, which is being supplied an href value and being rendered as an anchor. This provides the following advantages: - The whole button is now an anchor tag. This make the clickable surface area of the link bigger, which can provide better UX given that the icon could be somewhat small previously - Even if the contents of the button get truncated, the link itself will always be accesible given that it's the button as a whole that acts as an anchor tag - Instead of relaying on a component in the contents of the button to render the external link icon, it is now being rendered via button props. This ensures that, even when the content of the button is truncated, the icon remains visible keeping users aware that they're about to click a link that will open in a new tab ### External link handling The way `useRedirectLink` was decorating onClick events was preventing external links to work properly. The event propagation chain was being stopped (`event.preventDefault`), which for regular links is required in order to prevent the page reload that browsers do by default on link redirections as we're in a SPA. But, for external links, we do want the anchor tag to handle the event so that a new tab can be opened. A simple fix would be to remove the onClick handler from the link completely. Sadly, that couldn't be done because the link click has a telemetry event linked to it which we're sending via the onClick handler. So, the solution proposed in this PR adds an extra new parameter `external` to the hook. The way this parameter work is: - If false, the hook works as before and the prevent default will be added as always - If true, the onClick event will add the telemetry event but it will avoid modifying the propagation chain. This allows external link behaviour to work as expected while keeping the telemetry as it was ## UI The UI changed slightly because we're now using a button. Everything looks mostly the same but the error type "badge" now as an onHover background color https://github.com/user-attachments/assets/44a4daa3-df4e-4cea-b1b4-20271eb0cdd5
1 parent 8c9e148 commit 1b41310

File tree

4 files changed

+33
-21
lines changed

4 files changed

+33
-21
lines changed

x-pack/platform/plugins/shared/dataset_quality/public/components/dataset_quality_details/quality_issue_flyout/failed_docs/columns.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,9 @@ export const getFailedDocsErrorsColumns = (): Array<EuiBasicTableColumn<FailedDo
4141
render: (_, { message }) => {
4242
return <ErrorMessage errorMessage={message} />;
4343
},
44+
mobileOptions: {
45+
width: '100%',
46+
},
4447
},
4548
{
4649
name: (

x-pack/platform/plugins/shared/dataset_quality/public/components/dataset_quality_details/quality_issue_flyout/failed_docs/error_stacktrace_link.tsx

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55
* 2.0.
66
*/
77

8-
import { EuiBadge, EuiLink } from '@elastic/eui';
8+
import { EuiButton } from '@elastic/eui';
9+
import type { UseEuiTheme } from '@elastic/eui';
910
import React from 'react';
1011
import { NavigationSource } from '../../../../services/telemetry';
1112
import { FAILURE_STORE_SELECTOR } from '../../../../../common/constants';
@@ -15,13 +16,11 @@ import {
1516
useRedirectLink,
1617
} from '../../../../hooks';
1718

18-
export const ErrorStacktraceLink = ({
19-
errorType,
20-
children,
21-
}: {
22-
errorType: string;
23-
children?: React.ReactNode;
24-
}) => {
19+
const ButtonStyles = ({ euiTheme }: UseEuiTheme) => ({
20+
fontWeight: euiTheme.font.weight.bold,
21+
});
22+
23+
export const ErrorStacktraceLink = ({ errorType }: { errorType: string }) => {
2524
const { datasetDetails, timeRange } = useDatasetQualityDetailsState();
2625
const query = { language: 'kuery', query: `error.type: "${errorType}"` };
2726
const { sendTelemetry } = useDatasetDetailsRedirectLinkTelemetry({
@@ -35,20 +34,25 @@ export const ErrorStacktraceLink = ({
3534
sendTelemetry,
3635
timeRangeConfig: timeRange,
3736
selector: FAILURE_STORE_SELECTOR,
37+
external: true,
3838
});
3939

4040
return (
41-
<EuiBadge color="hollow">
42-
<strong>{errorType}</strong>
43-
<EuiLink
44-
external
45-
{...linkProps}
46-
color="primary"
47-
data-test-subj={`datasetQualityTableDetailsLink-${datasetDetails.name}`}
48-
target="_blank"
49-
>
50-
{children}
51-
</EuiLink>
52-
</EuiBadge>
41+
<EuiButton
42+
color="text"
43+
iconType="popout"
44+
iconSide="right"
45+
target="_blank"
46+
size="s"
47+
role="link"
48+
element="a"
49+
data-test-subj={`datasetQualityTableDetailsLink-${datasetDetails.name}`}
50+
textProps={{
51+
css: ButtonStyles,
52+
}}
53+
{...linkProps}
54+
>
55+
{errorType}
56+
</EuiButton>
5357
);
5458
};

x-pack/platform/plugins/shared/dataset_quality/public/components/dataset_quality_details/quality_issue_flyout/failed_docs/filed_info.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ export const FailedFieldInfo = () => {
7171
<EuiHorizontalRule margin="xs" />
7272
<EuiBasicTable
7373
tableLayout="fixed"
74+
responsiveBreakpoint={true}
7475
columns={failedDocsErrorsColumns}
7576
items={renderedFailedDocsErrorsItems ?? []}
7677
loading={isFailedDocsErrorsLoading}

x-pack/platform/plugins/shared/dataset_quality/public/hooks/use_redirect_link.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,15 @@ export const useRedirectLink = <T extends BasicDataStream | string>({
2424
breakdownField,
2525
sendTelemetry,
2626
selector,
27+
external = false,
2728
}: {
2829
dataStreamStat: T;
2930
query?: Query | AggregateQuery;
3031
timeRangeConfig: TimeRangeConfig;
3132
breakdownField?: string;
3233
sendTelemetry: SendTelemetryFn;
3334
selector?: DataStreamSelector;
35+
external?: boolean;
3436
}) => {
3537
const {
3638
services: { share },
@@ -55,7 +57,8 @@ export const useRedirectLink = <T extends BasicDataStream | string>({
5557

5658
const onClickWithTelemetry = (event: Parameters<RouterLinkProps['onClick']>[0]) => {
5759
sendTelemetry();
58-
if (config.routerLinkProps.onClick) {
60+
// If the link is external, we don't want to use router link props handler because it would prevent the default behavior of the event and a new tab wouldn't be opened.
61+
if (config.routerLinkProps.onClick && !external) {
5962
config.routerLinkProps.onClick(event);
6063
}
6164
};
@@ -82,6 +85,7 @@ export const useRedirectLink = <T extends BasicDataStream | string>({
8285
breakdownField,
8386
selector,
8487
sendTelemetry,
88+
external,
8589
]);
8690
};
8791

0 commit comments

Comments
 (0)