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

Jump to bottom

fix: EditableText and EditableHeading overflowing in the catalog #2555

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

fix: EditableText and EditableHeading overflowing in the catalog #2555

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0042c7f
fix: EditableText and EditableHeading overflowing in the catalog
mashazyu Oct 20, 2024
2160a78
fix: .editableTypography set display to block and add missing 6px to …
mashazyu Oct 21, 2024
d326d8d
fix: revert display of .editableTypography to inline-flex to fix jump…
mashazyu Nov 6, 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
3 changes: 2 additions & 1 deletion packages/core/src/components/EditableTypography/EditableTypography.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@
.editableTypography {
display: inline-flex;
min-width: 0;
max-width: 100%;
// Shifts the component to align the text with the container
margin-left: -6px;
// Adds 6 px removed above to the element width to avoid unnecessary overflow
max-width: calc(100% + 6px);
Copy link
Author

@mashazyu mashazyu Oct 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

question:
@talkor

First of all, thank you very much for the quick feedback and pointing out accessibility issues.

I didn't go with "display: block" initially because it didn't fix the overflowing issue. I looked further and realized that what's causing this issue is this negative margin-left in combination with max-width set to 100%. Adding missing 6px to max-width fixes the issue without chaning the display value.

Wdyt about this approach to fixing this issue and do you think the display value still needs to be updated? Or maybe I should also look into how element is centered?

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @mashazyu ! Looks like a great solution. Since there's a change in the component we'll need to test it a bit to make sure it doesn't break anything. Assuming everything is ok I'll merge it. In the meantime I've added the hacktoberfest accepted tag. Thank you so much for this!

overflow: hidden;
position: relative;

Expand Down