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

Addon-docs: Fix newline handling in ArgsTable code blocks #12882

Conversation

niklasmh
Copy link
Contributor

Issue: #11358 (propTypes) and #12848 (interface)

What I did

Added white-space: pre-wrap; on code blocks (specifically). This does not affect inline code.

Before:

image

After:

image

How to test

  • Is this testable with Jest or Chromatic screenshots?
    • No, unless ArgsTables are included in the screenshots
  • Does this need a new example in the kitchen sink apps?
    • Yes
    • Added a propTypes example in cra-kitchen-sink\src\components\Container.js together with an ArgsTable in the test story: cra-kitchen-sink\src\stories\test.stories.mdx.
    • Added an interface example in cra-ts-kitchen-sink\src\components\Button.tsx.
  • Does this need an update to the documentation?
    • No

If your answer is yes to any of these, please make sure to include it in your PR.

@niklasmh
Copy link
Contributor Author

This is how the examples that were added looks like:

For the cra-kitchen-sink app:

image

For the cra-ts-kitchen-sink app:

image

@stale
Copy link

stale bot commented Dec 25, 2020

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@stale stale bot added the inactive label Dec 25, 2020
@stale stale bot removed the inactive label Jul 12, 2021
@shilman shilman added this to the 6.4 PRs milestone Jul 22, 2021
@MichaelArestad
Copy link
Contributor

It looks like the code block is missing styles. Compare the inline code examples to the block:

image

@nx-cloud
Copy link

nx-cloud bot commented Aug 3, 2021

Nx Cloud Report

CI ran the following commands for commit bd5889f. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

Status Command
#000000 nx run-many --target=prepare --all --parallel --max-parallel=15

Sent with 💌 from NxCloud.

@niklasmh
Copy link
Contributor Author

niklasmh commented Aug 3, 2021

Thanks for the feedback, @MichaelArestad!

Do you mean making the inline code (here in red) to become a single block when it breaks, like the block in the green area?

image

@MichaelArestad
Copy link
Contributor

@niklasmh Not quite. I'm referring for the differences in styles between this:

image

and this:
image

It's somewhat tricky to spot, but you'll see they actually have different font and color styles.

@niklasmh
Copy link
Contributor Author

niklasmh commented Aug 4, 2021

@MichaelArestad Nice spotted! I can attempt to do something about it. I assume the first style is preferred? As it is the default for inline code.

@MichaelArestad
Copy link
Contributor

@niklasmh That would be great. Yes. The styles for the inline code are preferred. Thank you!

Setting smaller font-size on code blocks is also done at GitHub
@niklasmh
Copy link
Contributor Author

@MichaelArestad As far I could find the only difference was the font-size which made the text appear darker. I also saw that setting a smaller font-size on inline code blocks is a normal practice (e.g. GitHub) - therefore it seems like a reasonable change to add.

Now it looks like this:
image

@MichaelArestad
Copy link
Contributor

Thank you for making these changes. This PR is so close! The only thing it's missing is a font-family declaration to match:

image

@niklasmh
Copy link
Contributor Author

Now the font should be good too! Thanks for guiding me through my first PR here at storybook, @MichaelArestad :)

@MichaelArestad
Copy link
Contributor

I'm seeing some visual diffs of changes that are unrelated to this PR. I suspect a rebase or merging an updated next branch will resolve those changes. As for the changes you made, they look great. Thank you!

@niklasmh niklasmh force-pushed the 11358-render-newlines-in-markdown-code-blocks-in-argtables branch from c7b62fd to c7cd395 Compare August 17, 2021 09:26
…11358-render-newlines-in-markdown-code-blocks-in-argtables
@MichaelArestad
Copy link
Contributor

@shilman I think this one is ready to 🚢

@shilman shilman changed the title Fix #11358 by adding pre-wrap on code blocks Addon-docs: Fix code blocks line wrapping Aug 20, 2021
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 @niklasmh !!! 🙏

@shilman shilman changed the title Addon-docs: Fix code blocks line wrapping Addon-docs: Fix line wrapping in ArgsTable code blocks Aug 20, 2021
@shilman shilman changed the title Addon-docs: Fix line wrapping in ArgsTable code blocks Addon-docs: Fix line rendering in ArgsTable code blocks Aug 20, 2021
@shilman shilman changed the title Addon-docs: Fix line rendering in ArgsTable code blocks Addon-docs: Fix newline handling in ArgsTable code blocks Aug 20, 2021
@shilman shilman merged commit 1ad300f into storybookjs:next Aug 20, 2021
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.

None yet

5 participants