Skip to content
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.

Sign up for GitHub

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

[Dashboard][Papercut] Customize overflow text for Links Panel #199844

Closed
wants to merge 21 commits into from
Closed

[Dashboard][Papercut] Customize overflow text for Links Panel #199844

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
b4ae7e0
initial commit
rshen91 Nov 12, 2024
b676aff
span:first-child
rshen91 Nov 13, 2024
8277b06
Merge remote-tracking branch 'upstream/main' into overlfow-text-link-…
rshen91 Nov 13, 2024
b6e11f2
fix external links
rshen91 Nov 13, 2024
68dfef4
type check
rshen91 Nov 13, 2024
0188e9f
Merge branch 'main' into overlfow-text-link-panel
rshen91 Nov 13, 2024
b4d644e
clean up
rshen91 Nov 13, 2024
46db6b8
Merge remote-tracking branch 'upstream/main' into overlfow-text-link-…
rshen91 Nov 13, 2024
9aa8e82
Merge remote-tracking branch 'origin/overlfow-text-link-panel' into o…
rshen91 Nov 13, 2024
775c949
type checks
rshen91 Nov 13, 2024
765ba9f
fix validation issue
rshen91 Nov 13, 2024
13ae8b5
Merge remote-tracking branch 'upstream/main' into overlfow-text-link-…
rshen91 Nov 13, 2024
617eb54
Merge branch 'main' into overlfow-text-link-panel
rshen91 Nov 14, 2024
461a43a
[CI] Auto-commit changed files from 'make api-docs'
kibanamachine Nov 14, 2024
a183945
Merge remote-tracking branch 'upstream/main' into overlfow-text-link-…
rshen91 Nov 14, 2024
1ac9d8d
update toggles ui code review
rshen91 Nov 14, 2024
8218344
Merge branch 'main' into overlfow-text-link-panel
rshen91 Nov 18, 2024
f93d22a
partial refactor
rshen91 Nov 18, 2024
cdad488
type check
rshen91 Nov 18, 2024
be4c8c1
types
rshen91 Nov 18, 2024
00c7384
wip
rshen91 Nov 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/plugins/links/public/components/dashboard_link/dashboard_link_component.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,8 @@ export const DashboardLinkComponent = ({
linkCurrent: link.destination === parentDashboardId,
dashboardLinkError: Boolean(link.error),
'dashboardLinkError--noLabel': !link.label,
overflowEllipsis: link.options?.overflowEllipsis,
overflowTextWrap: link.options?.overflowTextWrap,
})}
label={linkLabel}
external={link.options?.openInNewTab}
Expand Down
6 changes: 5 additions & 1 deletion src/plugins/links/public/components/external_link/external_link_component.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* License v3.0 only", or the "Server Side Public License, v 1".
*/

import classNames from 'classnames';
import React, { useMemo } from 'react';
import { EuiListGroupItem } from '@elastic/eui';
import { METRIC_TYPE } from '@kbn/analytics';
Expand Down Expand Up @@ -51,7 +52,10 @@ export const ExternalLinkComponent = ({
external
color="text"
isDisabled={Boolean(link.error)}
className={'linksPanelLink'}
className={classNames('linksPanelLink', {
overflowEllipsis: linkOptions?.overflowEllipsis,
overflowTextWrap: linkOptions?.overflowTextWrap,
})}
showToolTip={Boolean(link.error)}
toolTipProps={{
content: link.error?.message,
Expand Down
15 changes: 14 additions & 1 deletion src/plugins/links/public/components/links_component.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,24 @@

&.linkCurrent {
border-radius: 0;
.euiListGroupItem__text {
&.euiListGroupItem__text {
cursor: default;
color: $euiColorPrimary;
}
}

&.overflowTextWrap span:first-child {
white-space: normal !important;
overflow: auto !important;
overflow-wrap: anywhere !important;
text-overflow: unset !important;
}

&.overflowTextEllipsis {
white-space: nowrap;
overflow: hidden;
text-overflow: ellipsis;
}
}

.verticalLayoutWrapper {
Expand Down
2 changes: 2 additions & 0 deletions src/plugins/links/server/content_management/schema/v1/cm_services.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ const dashboardLinkSchema = schema.object({
openInNewTab: schema.boolean(),
useCurrentFilters: schema.boolean(),
useCurrentDateRange: schema.boolean(),
overflowTextWrap: schema.boolean(),
overflowEllipsis: schema.boolean(),
},
{ unknowns: 'forbid' }
)
Expand Down
28 changes: 28 additions & 0 deletions ...tation_util/public/components/dashboard_drilldown_options/dashboard_drilldown_options.tsx
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the problem with making changes to this file is that this component is used for both drilldowns and links. You can see here that we now have an "Overflow" setting for dashboard drilldowns which.... doesn't really make sense :P

Screenshot 2024-11-14 at 8 53 36 AM

This setting is only relevant to links panels IMO.

Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,34 @@ export const DashboardDrilldownOptionsComponent = ({
onChange={() => onOptionChange({ openInNewTab: !options.openInNewTab })}
data-test-subj="dashboardDrillDownOptions--openInNewTab--checkbox"
/>
<EuiSpacer size="s" />
<EuiSwitch
compressed
name="overflowEllipsis"
label={dashboardDrilldownConfigStrings.component.getOverflowEllipsis()}
checked={options.overflowEllipsis}
onChange={() =>
onOptionChange({
overflowEllipsis: !options.overflowEllipsis,
overflowTextWrap: !options.overflowTextWrap,
})
}
data-test-subj="dashboardDrillDownOptions--overflowEllipsis--checkbox"
/>
<EuiSpacer size="s" />
<EuiSwitch
compressed
name="overflowTextWrap"
label={dashboardDrilldownConfigStrings.component.getOverflowTextWrap()}
checked={options.overflowTextWrap}
onChange={() =>
onOptionChange({
overflowTextWrap: !options.overflowTextWrap,
overflowEllipsis: !options.overflowEllipsis,
})
}
data-test-subj="dashboardDrillDownOptions--overflowTextWrap--checkbox"
/>
Copy link
Contributor

@Heenawter Heenawter Nov 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rshen91 I think it is confusing to have seperate "Wrap text" and "Ellipses" toggles - these components should be used for entirely distinct settings, and they shouldn't be "linked" (pun intended 😛) together like this:

Nov-14-2024 08-12-19

I think a button group, where only a single option is allowed, might be a better option here. Pinging @andreadelrio here to see if she has a better suggestion, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call! I updated the UI in 1ac9d8d. I'm open to suggestions with any labels and naming thanks!!!

Copy link
Contributor

@Heenawter Heenawter Nov 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we keep the setting per-link, I think the "Overflow" setting should come first:

Current Suggested
image image

I also wonder if we could do something with an icon instead? This is just an idea, though - would definitely need @andreadelrio's opinion here 😆

Nov-14-2024 09-47-48

This ties in to #199844 (comment), since we shouldn't be putting this setting in the shared component - it should be just for links.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to using a button group.

</div>
</EuiFormRow>
</>
Expand Down
4 changes: 4 additions & 0 deletions src/plugins/presentation_util/public/components/dashboard_drilldown_options/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,14 @@ export type DashboardDrilldownOptions = {
useCurrentFilters: boolean;
useCurrentDateRange: boolean;
openInNewTab: boolean;
overflowTextWrap: boolean;
overflowEllipsis: boolean;
Copy link
Contributor

@Heenawter Heenawter Nov 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be a setting for the entire links panel, not per-link. What do you think @rshen91?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @andreadelrio here, too 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally like having this per link, for greater user flexibility, but will default to what you both think if you both like it for the entire links panel.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andreadelrio can be our tie breaker, hahaha :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be a setting for the entire links panel.

};

export const DEFAULT_DASHBOARD_DRILLDOWN_OPTIONS: DashboardDrilldownOptions = {
openInNewTab: false,
useCurrentDateRange: true,
useCurrentFilters: true,
overflowEllipsis: true,
overflowTextWrap: false,
};
8 changes: 8 additions & 0 deletions src/plugins/presentation_util/public/i18n/dashboard_drilldown_config.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,13 @@ export const dashboardDrilldownConfigStrings = {
i18n.translate('presentationUtil.dashboardDrilldownConfig.components.openInNewTab', {
defaultMessage: 'Open dashboard in new tab',
}),
getOverflowTextWrap: () =>
i18n.translate('presentationUtil.dashboardDrilldownConfig.components.overflowTextWrap', {
defaultMessage: 'Wrap text',
}),
getOverflowEllipsis: () =>
i18n.translate('presentationUtil.dashboardDrilldownConfig.components.overflowEllipsis', {
defaultMessage: 'Truncate text with ellipsis',
}),
},
};
28 changes: 28 additions & 0 deletions ..._enhanced/public/drilldowns/url_drilldown/components/url_drilldown_collect_config/i18n.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,31 @@ export const txtUrlTemplateEncodeDescription = i18n.translate(
defaultMessage: 'If enabled, URL will be escaped using percent encoding',
}
);

export const txtUrlTemplateOverflowTextWrap = i18n.translate(
'uiActionsEnhanced.drilldowns.urlDrilldownCollectConfig.overflowTextWrap',
{
defaultMessage: 'Wrap text',
}
);

export const txtUrlTemplateOverflowTextWrapDescription = i18n.translate(
'uiActionsEnhanced.drilldowns.urlDrilldownCollectConfig.overflowTextWrapDescription',
{
defaultMessage: 'If enabled, text will wrap instead of being truncated',
}
);

export const txtUrlTemplateOverflowEllipsis = i18n.translate(
'uiActionsEnhanced.drilldowns.urlDrilldownCollectConfig.overflowEllipsis',
{
defaultMessage: 'Truncate text with ellipsis',
}
);

export const txtUrlTemplateOverflowEllipsisDescription = i18n.translate(
'uiActionsEnhanced.drilldowns.urlDrilldownCollectConfig.overflowEllipsisDescription',
{
defaultMessage: 'If enabled, text will be truncated with ellipsis',
}
);
2 changes: 2 additions & 0 deletions ...ic/drilldowns/url_drilldown/components/url_drilldown_collect_config/test_samples/demo.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ export const Demo = () => {
openInNewTab: false,
encodeUrl: true,
url: { template: '' },
overflowEllipsis: true,
overflowTextWrap: false,
});

return (
Expand Down
50 changes: 50 additions & 0 deletions ...rilldowns/url_drilldown/components/url_drilldown_collect_config/url_drilldown_options.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ import {
txtUrlTemplateEncodeDescription,
txtUrlTemplateEncodeUrl,
txtUrlTemplateOpenInNewTab,
txtUrlTemplateOverflowEllipsis,
txtUrlTemplateOverflowEllipsisDescription,
txtUrlTemplateOverflowTextWrap,
txtUrlTemplateOverflowTextWrapDescription,
} from './i18n';
import { UrlDrilldownOptions } from '../../types';

Expand Down Expand Up @@ -55,6 +59,52 @@ export const UrlDrilldownOptionsComponent = ({
onChange={() => onOptionChange({ encodeUrl: !options.encodeUrl })}
data-test-subj="urlDrilldownEncodeUrl"
/>
<EuiSpacer size="s" />
<EuiSwitch
compressed
id="overflowEllipsis"
name="overflowEllipsis"
label={
<>
{txtUrlTemplateOverflowEllipsis}
<EuiSpacer size={'s'} />
<EuiTextColor color="subdued">
{txtUrlTemplateOverflowEllipsisDescription}
</EuiTextColor>
</>
}
checked={options.overflowEllipsis}
onChange={() =>
onOptionChange({
overflowEllipsis: !options.overflowEllipsis,
overflowTextWrap: !options.overflowTextWrap,
})
}
data-test-subj="urlDrilldownOverflowEllipsis"
/>
<EuiSpacer size="s" />
<EuiSwitch
compressed
id="overflowTextWrap"
name="overflowTextWrap"
label={
<>
{txtUrlTemplateOverflowTextWrap}
<EuiSpacer size={'s'} />
<EuiTextColor color="subdued">
{txtUrlTemplateOverflowTextWrapDescription}
</EuiTextColor>
</>
}
checked={options.overflowTextWrap}
onChange={() =>
onOptionChange({
overflowTextWrap: !options.overflowTextWrap,
overflowEllipsis: !options.overflowEllipsis,
})
}
data-test-subj="urlDrilldownOverflowTextWrap"
/>
</div>
</EuiFormRow>
</>
Expand Down
2 changes: 2 additions & 0 deletions src/plugins/ui_actions_enhanced/public/drilldowns/url_drilldown/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,6 @@ import { UrlDrilldownOptions } from './types';
export const DEFAULT_URL_DRILLDOWN_OPTIONS: UrlDrilldownOptions = {
encodeUrl: true,
openInNewTab: true,
overflowTextWrap: false,
overflowEllipsis: true,
};
2 changes: 2 additions & 0 deletions src/plugins/ui_actions_enhanced/public/drilldowns/url_drilldown/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ export type UrlDrilldownConfig = {
export type UrlDrilldownOptions = {
openInNewTab: boolean;
encodeUrl: boolean;
overflowTextWrap: boolean;
overflowEllipsis: boolean;
};

/**
Expand Down
6 changes: 6 additions & 0 deletions ...s/drilldowns/embeddable_to_dashboard_drilldown/embeddable_to_dashboard_drilldown.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ describe('.isConfigValid()', () => {
useCurrentDateRange: false,
useCurrentFilters: false,
openInNewTab: false,
overflowEllipsis: true,
overflowTextWrap: false,
})
).toBe(false);
});
Expand All @@ -36,6 +38,8 @@ describe('.isConfigValid()', () => {
useCurrentDateRange: false,
useCurrentFilters: false,
openInNewTab: false,
overflowEllipsis: true,
overflowTextWrap: false,
})
).toBe(true);
});
Expand Down Expand Up @@ -111,6 +115,8 @@ describe('.execute() & getHref', () => {
useCurrentFilters: false,
useCurrentDateRange: false,
openInNewTab: false,
overflowEllipsis: true,
overflowTextWrap: false,
...config,
};

Expand Down
Loading