[APM] ellipsis truncation issue - dependencies and service section #122203

gayathrir1172 · 2022-01-03T07:00:06Z

Summary

This PR will resolve ellipsis truncation issue which is mentioned here. The following screenshots clearly show that the ellipsis has been applied on dependency and service name section.

Fixes #112976

Before:

After:

Dependencies section :

Services section:

For maintainers

This was checked for breaking API changes and was labeled appropriately

Apm truncate issue 112976

kibanamachine · 2022-01-03T07:00:08Z

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

elasticmachine · 2022-01-03T07:00:10Z

Pinging @elastic/apm-ui (Team:apm)

cla-checker-service · 2022-01-03T07:00:13Z

💚 CLA has been signed

gayathrir1172 · 2022-01-03T08:02:43Z

I have signed in the contributor agreement

gayathrir1172 · 2022-01-05T05:00:50Z

Hi @sqren can you please review this PR ?

kpatticha

Thanks for you contribution @gayathrir1172 . The change looks good to me but the PR is a bit outdated and as a result one of the checks is failing.

You can easily update your PR by adding the following @elasticmachine merge upstream as a comment

gayathrir1172 · 2022-01-05T10:36:22Z

@elasticmachine merge upstream

gayathrir1172 · 2022-01-05T10:48:10Z

Thanks @kpatticha for reviewing the PR. I tried commenting @elasticmachine merge upstream, but still the PR seems to be outdated. Can you please help here ?

kpatticha · 2022-01-05T12:59:57Z

@elasticmachine merge upstream

kpatticha · 2022-01-05T13:12:48Z

sorry for the inconvenience, it seems that bot doesn't respond. I'm not sure why 🤔

Can you please try to rebase and update your branch manually?

kpatticha · 2022-01-05T13:57:02Z

Also, in my screen (16-inch) found that dependency name and the service name overflows

The text-overflow property doesn't force an overflow to occur. To make text overflow its container you have to set the overflow css property

gayathrir1172 · 2022-01-05T16:01:26Z

Hey @kpatticha
a quick question, are you using miqdigital:112976 branch to check this ?

kpatticha · 2022-01-06T08:23:07Z

@gayathrir1172 I fetched this PR (with the changes) locally and tested.

Taking current pull from kibana main

gayathrir1172 · 2022-01-06T09:57:32Z

Hey @kpatticha the PR is not outdated now, I have taken the current pull.
Will look into the truncation issue which you have reported and get back to you.

…pm-truncate-issue-112976

kpatticha · 2022-03-28T13:03:05Z

x-pack/plugins/apm/public/components/shared/backend_link.tsx


-const StyledLink = euiStyled(EuiLink)`${truncate('100%')};`;
+const StyledLink = euiStyled(EuiLink)`min-width: 0`;


What's the purpose of min-width if is set to 0?

Hey @kpatticha, the min width is required here because it is a flex item. Flex items takes content width by default, so giving min-width is necessary here.
Reference : https://css-tricks.com/flexbox-truncated-text/

x-pack/plugins/apm/public/components/shared/backend_link.tsx

kpatticha · 2022-03-28T13:22:05Z

x-pack/plugins/apm/public/components/shared/backend_link.tsx

+const truncateAnchorClassname = '_apm_truncate_anchor_';
+
+const TruncationWrapper = euiStyled.div`
+  width: 50px;


How do you determine the width?

Here, some fixed width is required for ellipsis truncation to work.
Reference : https://css-tricks.com/snippets/css/truncate-string-with-ellipsis/

The <BackendLink /> is a shared component, there might be an issue by setting the width:50px.
Can you please make sure how it affects the components that use <BackendLink />

Hey @kpatticha I have already checked this, I do not find this affects other components. Only service and dependency sections use this, and I have handled the scenarios accordingly.

One quick and easy way to find where the component is used, you can run the following command from APM folder (kibana/x-pack/plugins/apm)
grep -rn "<BackendLink" .

./public/components/app/backend_inventory/backend_inventory_dependencies_table/index.tsx:66: <BackendLink ./public/components/app/service_overview/service_overview_dependencies_table/index.tsx:85: <BackendLink ./public/components/app/transaction_details/waterfall_with_summary/waterfall_container/waterfall/span_flyout/sticky_span_properties.tsx:108: <BackendLink ./public/components/shared/backend_link.stories.tsx:30: return <BackendLink {...args} />;

The given width works for the dependency section in the service overview page (services/SERVICE_NAME/overview) but when I check the dependencies table while I have enough space, it truncates the dependency name

The same applies for the span_flyout.

Screen.Recording.2022-03-31.at.14.53.18.mov

kpatticha · 2022-03-28T13:22:22Z

x-pack/plugins/apm/public/components/shared/truncate_with_tooltip/index.tsx

@@ -45,3 +45,9 @@ export function TruncateWithTooltip(props: Props) {
    </TooltipWrapper>
  );
 }
+
+export function TruncateWithoutTooltip(props: Props) {


can you please help me understanding why we need another component without a tooltip?

@kpatticha We need another component here because earlier truncation was happening at group level. Now, I have modified it in a way that truncation would be applied on item level.

created a new component for truncation without tooltip

gayathrir1172 · 2022-03-31T05:27:07Z

@kpatticha I have updated the PR with the file name changes you have mentioned. Kindly check. Thanks !

cauemarcondes

@gayathrir1172 thanks for your work on this issue. I have added a small comment. take a look and let me know what you think.

cauemarcondes · 2022-04-05T14:33:59Z

x-pack/plugins/apm/public/components/shared/service_link.tsx

+        <EuiFlexItem>
+          <TruncationWrapper>
+            <TruncateWithoutTooltip text={serviceName} content={serviceName} />
+          </TruncationWrapper>
+        </EuiFlexItem>


@gayathrir1172 It looks like the problem described on this issue is fixed by only changing this part of code:

Suggested change

<EuiFlexItem>

<TruncationWrapper>

<TruncateWithoutTooltip text={serviceName} content={serviceName} />

</TruncationWrapper>

</EuiFlexItem>

<EuiFlexItem className="eui-textTruncate">

<span className="eui-textTruncate">{serviceName}</span>

</EuiFlexItem>

You only need to apply the same change at public/components/shared/backend_link.tsx

Hey @cauemarcondes sure, will try as you have suggested and update the PR. Thanks !

Hey @cauemarcondes I have implemented as you have suggested. Please check. thanks !

Apm truncation

cauemarcondes

LGTM, thanks for your help @gayathrir1172

gayathrir1172 · 2022-05-03T13:05:21Z

@kpatticha can you please review and approve this PR ? And if you are also good with the code changes, can you please merge this PR too ? Thanks !

kpatticha · 2022-05-03T14:16:16Z

@gayathrir1172 thanks for your contribution 👍 . Once the CI passes it will be merged

cauemarcondes · 2022-05-04T12:56:56Z

@elasticmachine merge upstream

gbamparop · 2022-05-04T13:37:10Z

@elasticmachine run elasticsearch-ci/docs

gbamparop · 2022-05-04T13:37:36Z

Buildkite, test this

cauemarcondes · 2022-05-04T13:38:01Z

@elasticmachine run elasticsearch-ci/docs

kibana-ci · 2022-05-04T14:43:11Z

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`apm`	2.8MB	2.8MB	+166.0B

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

…lastic#122203) * Added ellipsis truncation to apm dependency * Added ellipsis truncation to apm service * Removed un-used variable * Added ellipsis truncation * removing unused import * modified css classname * created a new component for truncation without tooltip * using text truncate styling and removed truncatewithouttooltip component * restored styledlink

gayathrir1172 added 4 commits December 28, 2021 12:02

Added ellipsis truncation to apm dependency

6aefa3e

Added ellipsis truncation to apm service

ec14954

Removed un-used variable

0a6b35b

Merge pull request #148 from gayathrir1172/apm-truncate-issue-112976

35e5190

Apm truncate issue 112976

gayathrir1172 requested a review from a team as a code owner January 3, 2022 07:00

botelastic bot added the Team:APM - DEPRECATED Use Team:obs-ux-infra_services. label Jan 3, 2022

kibanamachine added the 💝community label Jan 3, 2022

gayathrir1172 mentioned this pull request Jan 3, 2022

[APM] Long dependency names are cut off without using ellipsis (...) #112976

Closed

gayathrir1172 changed the title ~~APM ellipsis truncation issue - dependencies and service section~~ [APM] ellipsis truncation issue - dependencies and service section Jan 5, 2022

kpatticha added release_note:fix v8.1.0 labels Jan 5, 2022

kpatticha reviewed Jan 5, 2022

View reviewed changes

Merge pull request #155 from elastic/main

cae1d3b

Taking current pull from kibana main

gayathrir1172 marked this pull request as draft January 7, 2022 08:03

gayathrir1172 added 3 commits February 21, 2022 18:33

Merge branch 'main' of https://github.com/gayathrir1172/kibana into a…

8d2e96b

…pm-truncate-issue-112976

Merge branch 'elastic:main' into apm-truncate-issue-112976

5fa2518

Merge branch 'elastic:main' into apm-truncate-issue-112976

40b7ffa

kpatticha reviewed Mar 28, 2022

View reviewed changes

x-pack/plugins/apm/public/components/shared/backend_link.tsx Outdated Show resolved Hide resolved

kpatticha reviewed Mar 28, 2022

View reviewed changes

gayathrir1172 added 3 commits March 29, 2022 16:07

created a new component for truncation without tooltip

6b08579

Merge pull request #202 from gayathrir1172/apm-truncate-issue-112976

bc7293a

created a new component for truncation without tooltip

Merge branch 'elastic:main' into 112976

87e9f78

cauemarcondes requested changes Apr 5, 2022

View reviewed changes

gayathrir1172 added 4 commits April 27, 2022 15:10

using text truncate styling and removed truncatewithouttooltip component

d8c804b

restored styledlink

8a7dc54

Merge pull request #203 from gayathrir1172/apm-truncate-issue-112976

4bf6866

Apm truncation

Merge branch 'elastic:main' into 112976

431a01b

cauemarcondes added v8.3.0 and removed v8.1.0 labels Apr 28, 2022

cauemarcondes approved these changes Apr 28, 2022

View reviewed changes

Merge branch 'elastic:main' into 112976

b6c02be

kpatticha approved these changes May 3, 2022

View reviewed changes

kpatticha enabled auto-merge (squash) May 3, 2022 14:15

kpatticha merged commit 4f88950 into elastic:main May 4, 2022

kibanamachine added the backport:skip This commit does not require backporting label May 4, 2022


		const StyledLink = euiStyled(EuiLink)`${truncate('100%')};`;
		const StyledLink = euiStyled(EuiLink)`min-width: 0`;

[APM] ellipsis truncation issue - dependencies and service section #122203

[APM] ellipsis truncation issue - dependencies and service section #122203

Conversation

gayathrir1172 commented Jan 3, 2022 • edited Loading

Summary

For maintainers

kibanamachine commented Jan 3, 2022

elasticmachine commented Jan 3, 2022

cla-checker-service bot commented Jan 3, 2022 • edited Loading

gayathrir1172 commented Jan 3, 2022

gayathrir1172 commented Jan 5, 2022

kpatticha left a comment

Choose a reason for hiding this comment

gayathrir1172 commented Jan 5, 2022

gayathrir1172 commented Jan 5, 2022 • edited Loading

kpatticha commented Jan 5, 2022

kpatticha commented Jan 5, 2022

kpatticha commented Jan 5, 2022 • edited Loading

gayathrir1172 commented Jan 5, 2022

kpatticha commented Jan 6, 2022

gayathrir1172 commented Jan 6, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gayathrir1172 commented Mar 31, 2022

cauemarcondes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cauemarcondes left a comment

Choose a reason for hiding this comment

gayathrir1172 commented May 3, 2022

kpatticha commented May 3, 2022

cauemarcondes commented May 4, 2022

gbamparop commented May 4, 2022

gbamparop commented May 4, 2022

cauemarcondes commented May 4, 2022

kibana-ci commented May 4, 2022

💚 Build Succeeded

Metrics [docs]

Async chunks

gayathrir1172 commented Jan 3, 2022 •

edited

Loading

cla-checker-service bot commented Jan 3, 2022 •

edited

Loading

gayathrir1172 commented Jan 5, 2022 •

edited

Loading

kpatticha commented Jan 5, 2022 •

edited

Loading