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

ArgsTable: Fix styles to allow long text to wrap #11818

Merged
merged 1 commit into from
Aug 17, 2020
Merged

ArgsTable: Fix styles to allow long text to wrap #11818

merged 1 commit into from
Aug 17, 2020

Conversation

cfhull
Copy link
Contributor

@cfhull cfhull commented Aug 5, 2020

Issue: #11362

What I did

This PR, while doing some awesome things, doesn't resolve the word wrapping issue.

Specifically, the new word-break: break-all property conflicts with the existing word-wrap: nowrap property that is inherited from styles.span. I also added a min-width to keep it from breaking on short words.

Current behavior:
image

New behavior:
image

How to test

  • Is this testable with Jest or Chromatic screenshots? Chromatic (but apparently I can't do that from a fork)
  • Does this need a new example in the kitchen sink apps? No
  • Does this need an update to the documentation? No

@@ -30,10 +30,11 @@ const Summary = styled.div<{ isExpanded?: boolean }>(({ isExpanded }) => ({
flexWrap: 'wrap',
alignItems: 'flex-start',
marginBottom: '-4px',
minWidth: 100,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This value is a bit arbitrary. Without it, a short string like default would wrap the lt to the next line. 100px min width seemed small enough to me that the column wouldn't be overly big if there were no defaults, but would keep relatively short values from breaking to a new line.

Copy link
Member

Choose a reason for hiding this comment

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

Fair

@ajkl2533
Copy link
Contributor

ajkl2533 commented Aug 6, 2020

@cfhull thank you I hit the same issue

@ndelangen
Copy link
Member

@shilman review?

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

Thanks @cfhull !!

@shilman shilman changed the title CSS fixes to allow long text to wrap ArgsTable: Fix styles to allow long text to wrap Aug 17, 2020
@shilman shilman merged commit f9fa895 into storybookjs:next Aug 17, 2020
Copy link
Member

@jimmyandrade jimmyandrade left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants