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: markdown story styling #1536

Merged
merged 4 commits into from
Mar 10, 2023
Merged

fix: markdown story styling #1536

merged 4 commits into from
Mar 10, 2023

Conversation

jinlee93
Copy link
Contributor

@jinlee93 jinlee93 commented Mar 9, 2023 •
edited
Loading

Summary:

  • due to removing default browser styling via TW preflight, the docs also lost styles and looks not great
    - seems a lot of the markdown files had their own tsx file, which was unnecessary, so dried those into one Documentation.tsx
  • readded lost browser styling into the Documentation component
  • also moves these markdown docs into Documentation folder

Test Plan:

  • Wrote automated tests
  • CI tests / new tests are not applicable
  • Manually tested my changes, but I want to keep the details secret
  • Manually tested my changes, and here are the details:

@github-actions
Copy link

github-actions bot commented Mar 9, 2023 •
edited
Loading

size-limit report 📦

Path Size
components 121.45 KB (0%)
styles 3.1 KB (0%)

@jinlee93 jinlee93 force-pushed the jlee/fixMarkdownStoryStyling branch from 90c2a02 to ca107f3 Compare March 9, 2023 08:57
@codecov
Copy link

codecov bot commented Mar 9, 2023 •
edited
Loading

Codecov Report

Merging #1536 (ed4fd1d) into next (eef822f) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             next    #1536   +/-   ##
=======================================
  Coverage   91.87%   91.87%           
=======================================
  Files         285      285           
  Lines        4294     4294           
  Branches      792      792           
=======================================
  Hits         3945     3945           
  Misses        324      324           
  Partials       25       25

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

jinlee93
jinlee93 commented Mar 10, 2023
View reviewed changes
.storybook/main.js
@@ -8,7 +8,6 @@
*/
module.exports = {
stories: [
'./components/GettingStarted/GettingStarted.stories.tsx',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

now in ../docs

@jinlee93 jinlee93 force-pushed the jlee/fixMarkdownStoryStyling branch from 34dcb96 to 9e959e5 Compare March 10, 2023 02:50
@jinlee93 jinlee93 marked this pull request as ready for review March 10, 2023 05:06
@jinlee93 jinlee93 requested a review from a team March 10, 2023 05:06
booc0mtaco
booc0mtaco approved these changes Mar 10, 2023
View reviewed changes
Copy link
Contributor

@booc0mtaco booc0mtaco left a comment

Choose a reason for hiding this comment

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

Looks good to me. We might have places where we can tighten things even more, but we can always revisit that too

.storybook/components/Docs/Guildelines/Documents.stories.tsx Outdated
Comment on lines 14 to 26
parameters: {
chromatic: { disableSnapshot: true },
axe: {
skip: true,
},
},
decorators: [
(Story) => (
<Documentation>
<Story />
</Documentation>
),
],
Copy link
Contributor

Choose a reason for hiding this comment

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

this could be a JSON in Docs/ that gets imported into each of the subsequent documentation pages, so that they all share the same grammar. DRY-ing the story config some

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh good idea. Placed it in the MarkdownWrapper.tsx file instead tho since it needs a tsx extension for the decorator

.storybook/components/Docs/Documentation.module.css Outdated
@@ -0,0 +1,105 @@
.documentation {
Copy link
Contributor

Choose a reason for hiding this comment

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

Another possible name is .markdown and the component is MarkDownWrapper or something... 🤔?

Copy link
Contributor Author

@jinlee93 jinlee93 Mar 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I'm down for MarkdownWrapper and .markdown

@jinlee93 jinlee93 merged commit 89eba6b into next Mar 10, 2023
@jinlee93 jinlee93 deleted the jlee/fixMarkdownStoryStyling branch March 10, 2023 23:42
anniehu4
anniehu4 reviewed Mar 13, 2023
View reviewed changes
.storybook/components/Docs/Guildelines/Documents.stories.tsx

export default {
title: 'Documentation/Guidelines',
...markdownStorybookOptions,
Copy link
Contributor

Choose a reason for hiding this comment

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

ooh this is nice 😍

.storybook/components/Docs/MarkdownWrapper.module.css
@@ -0,0 +1,105 @@
.markdown {
Copy link
Contributor

Choose a reason for hiding this comment

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

late to review! but we should add a file comment explaining

  • purpose of this file
  • where the styles come from
  • why we don't care that it's all magic numbers

@booc0mtaco booc0mtaco mentioned this pull request Mar 17, 2023
Merged
booc0mtaco added a commit that referenced this pull request Mar 17, 2023
@booc0mtaco
Merge pull request #1549 from chanzuckerberg/release-v11.0.0
b52eace 
## [11.0.0](v10.0.0...v11.0.0) (2023-03-17)


### ⚠ BREAKING CHANGES

* add `indeterminate` prop to <Checkbox> that's separate from `checked` (#1520)

### Features

* add `indeterminate` prop to <Checkbox> that's separate from `checked` ([#1520](#1520)) ([d8e2cc4](d8e2cc4))
* **LoadingIndicator:** extract and use SVG animation directly ([#1540](#1540)) ([6e315ea](6e315ea))
* **menu:** add Menu.PlainButton as a minimally styled Menu button ([#1516](#1516)) ([8268d8e](8268d8e))


### Bug Fixes

* actually use our shared prettier config ([c98ea51](c98ea51))
* **Avatar:** loosen props for avatar aria-label component ([#1544](#1544)) ([4ab9183](4ab9183))
* markdown story styling ([#1536](#1536)) ([89eba6b](89eba6b))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anniehu4 anniehu4 anniehu4 left review comments

@booc0mtaco booc0mtaco booc0mtaco approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jinlee93 @booc0mtaco @anniehu4