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.

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

DlData: prevent text overflow from being visible in padding #1110

Merged
merged 1 commit into from
Dec 17, 2024

Conversation

matthew-white
Copy link
Member

This PR fixes the issue described at getodk/central#670 (comment). In the image in that comment, you can see that an ellipsis appears at the end of the third line of text, yet text is also shown below the ellipsis. In other words, the text is visible in the bottom padding of the <dd> element.

The <dd> element of DlData specifies overflow: hidden, which might be why the text doesn't overflow into the next row. However, overflow: hidden isn't enough to prevent the text from overflowing into the bottom padding.

We have used <dd> + the line-clamp mixin for a while, in EntityData. However, there, the <dd> element doesn't have padding, which is why this hasn't been an issue in the past. This is an issue only as of #1077, since that PR added padding to <dd> elements in .dl-horizontal. To be clear, I still think it's the right approach to add padding to <dd> elements, but I wanted to explain why we're only seeing this issue in hover cards.

The way I'm solving this issue is by wrapping the text in a <div> without padding. I'm then adding line-clamp and the tooltip to the <div> rather than the <dd>. Since the <div> doesn't have padding, its text won't overflow into the padding. The <dd> will still be free to have padding.

What has been done to verify that this works as intended?

I tried it out locally.

From looking on the QA server, I think the value shown in the image is "54.6029931 18.2311011 72.70000457763672 23.649". Note that there are no newline characters in the value. The value is just wrapping because it's long.

Why is this the best possible solution? Were any other approaches considered?

Instead of adding an inner element (the <div> in this case), others have tried fixing the height of the outer element or adding a pseudo-element. I think adding a <div> is the clearest approach.

Another idea entirely would have been to prevent the text from wrapping at all. We could truncate the text at the end of the first line. However, more data will be visible if we allow the text to wrap a little, which I think is nice.

Before submitting this PR, please make sure you have:

  • run npm run test and npm run lint and confirmed all checks still pass OR confirm CircleCI build passes
  • verified that any code or assets from external sources are properly credited in comments or that everything is internally sourced

@matthew-white matthew-white requested a review from ktuite December 16, 2024 20:09
@matthew-white matthew-white merged commit eea4324 into master Dec 17, 2024
1 check passed
@matthew-white matthew-white deleted the dl-data-overflow branch December 17, 2024 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants