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

Increase line-height in HoverCard to prevent text truncation #1112

Merged
merged 2 commits into from
Dec 17, 2024

Conversation

matthew-white
Copy link
Member

This PR attempts to address the issue described at getodk/central#670 (comment).

I actually had difficulty reproducing the issue locally. I can see that Szymon is seeing a different font than I am. However, after trying each of the fonts listed in the CSS, I don't see text truncation:

font-family: "Helvetica Neue", Helvetica, Arial, sans-serif;

I tried in both Chrome and Firefox.

At some point, we should maybe look into what options we have around standardizing fonts in Central. For now, I'm hoping I can solve the issue by increasing the line height. I strongly suspect that the text truncation that Szymon is seeing is due to line-height being set to 1 in the HoverCard component.

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

I wasn't able to reproduce the issue, so my main goal was to keep the look of the hover cards as consistent as possible while also increasing the line height. I compared hover cards side-by-side before/after making this change.

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

The main change I want to try is increasing the line height. After making that change, I also had to fiddle with margin/padding.

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 21:57
line-height: 1;
padding: $padding-panel-body;
padding-bottom: 12px;
line-height: 1.2;
Copy link
Member Author

Choose a reason for hiding this comment

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

This is still less than the default, but it's a number that we use in several places in Frontend.

@matthew-white matthew-white merged commit 4120f78 into master Dec 17, 2024
1 check passed
@matthew-white matthew-white deleted the hover-card-line-height branch December 17, 2024 17:38
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