feat(ui): Storybook – more global components & content for typography page #28680

vuluongj20 · 2021-09-20T18:28:35Z

1. Added more global components to Storybook (in docs-ui/components):

Code: a component for custom code samples, and an accompanying icon file (in app/icons/iconCode.tsx)

Sample: a visual wrapper for component examples

Table: styled table headers, rows and cells

TableOfContents: an interactive table of contents that highlights sections as users scroll through docs pages. This gets added to DocsPages automatically.

2. Added content to the Typography page
This includes 2 files, both in docs-ui/stories/core: typography.stories.mdx, and typography.tsx. The latter stores type definitions and exports table components for view in the docs page.

1. Added global components to Storybook, in `docs-ui/components`: - Code: a component for custom code samples, and a new icon file (in `app/icons/iconCode.tsx`) - Sample: a visual wrapper for component examples - Table: styled table rows and cells - TableOfContents: an interactive table of contents that highlights sections as users scroll through docs pages 2. Added content to the Typography page. This includes 2 files, both in `docs-ui/stories/core`: `typography.stories.mdx`, and `typography.tsx`. The latter stores type definitions and exports table components for view in the docs page.

evanpurkhiser · 2021-09-20T18:29:22Z

docs-ui/components/code.tsx

+import {Theme} from 'app/utils/theme';
+
+type Props = {
+	theme: Theme;


Do you have prettier enabled? Looks like you're using tabs instead of spaces here. Prettier should automatically fix this for you.

Yeah, but there's always some code editor conflicts with my other projects. Fixing this prompto

Are you using VSCode? There should be some way to have configuration per project with it.

Sublime Text. There's a plugin called EditorConfig that could fix this, but it requires adding a .editorconfig file to the project, which sounds annoying cause I have to ignore it for every commit

You could always add it to your personal global gitignore :) https://medium.com/@peter_graham/how-to-create-a-local-gitignore-1b19f083492b

woah learn something new every day 🌈

To avoid duplicate selector warning from stylelint.

billyvg · 2021-09-20T20:09:42Z

docs-ui/components/code.tsx

+
+type State = {};
+
+class Code extends Component<Props, State> {


Should these be written with functional components + hooks now?

Yepp, will update it.
Initially wrote this with the intention to add some interactive features that will require a state. But it seems that many of those features are already available in Storybook preview.

You can also easily do state using hooks these days! useState

evanpurkhiser · 2021-09-21T16:51:25Z

docs-ui/components/code.tsx

+  /**
+   * Main code content gets passed as the children prop
+   */
+  children: string;


Should this be React.ReactNode so it could be a bit more flexible?

Good point. Just tried inserting some non-text elements as children, and Prism converted all of them into plain text (Hello becomes 'Hello'), so I don't expect people to do this. Still, we should probably use React.ReactNode to account for when they do.

It might make some other parts of typing harder. So I would be OK requiring it to be a string

Now that I see what this is doing, I think it's OK

evanpurkhiser · 2021-09-21T16:54:00Z

docs-ui/components/code.tsx

+  function copyCode() {
+    const copyButton = copyButtonRef?.current;
+
+    if (copyButton) {


You could inverse this condition and make it a guard statement, which would reduce a level of indentation :)

function copyCode() { const copyButton = copyButtonRef?.current; if (!copyButton) { return } ... }

Ahh that does look nicer!

evanpurkhiser · 2021-09-21T16:55:38Z

docs-ui/components/code.tsx

+
+      copy(copiableContent);
+
+      copyButton.innerText = 'Copied';


Would it be better to have a useState({text, disabled}) and have a setState call here to update. Instead of digging into the DOM like this

You're right :) Fixing it

evanpurkhiser · 2021-09-21T16:56:38Z

docs-ui/components/code.tsx

+      <HighlightedCode className={className} ref={codeRef}>
+        {children}
+      </HighlightedCode>
+      <CopyButton ref={copyButtonRef} onClick={() => copyCode()}>


No need for the call wrapping, this can just be onClick={copyCode}. I might even suggest renaming copyCode to handleCopyCode

evanpurkhiser · 2021-09-21T16:57:22Z

docs-ui/components/code.tsx

+export default withTheme(Code);
+
+const Wrap = styled('pre')`
+  /** Increase specificity to override default styles */


evanpurkhiser · 2021-09-21T16:58:34Z

docs-ui/components/colorChip.tsx

+};
+
+type WrapperProps = {
+  large: boolean;


Would a size prop be more appropriate as like size="md" or size="lg"

Considered that but thought that since there are only two sizes, a boolean might work better. Could just use size instead if it doesn't make too much sense for you.

The reason I might suggest size, is in the case where we add more. Just gives that extra bit of flexability

docs-ui/components/sample.tsx

evanpurkhiser · 2021-09-21T17:01:08Z

.storybook/preview.tsx

 import ColorChip from '../docs-ui/components/colorChip';
 import DocsLinks from '../docs-ui/components/docsLinks';
 import DoDont from '../docs-ui/components/doDont';
+import Sample from '../docs-ui/components/sample';
+import TableOfContents from '../docs-ui/components/tableOfContents';


btw I added the docs-ui/... alias in webpack, so these could just be import ... from 'docs-ui/...'.

Might look marginally better

Oh I left a comment on your PR about how you used three dots (.../docs-ui) instead of two. Was waiting for you to fix it before using the alias. My bad for not following up on this earlier 😄

I'll just fix it in this PR

ohhhhhh lol that's my bad

evanpurkhiser · 2021-09-21T17:03:40Z

docs-ui/components/tableOfContents.tsx

+  margin-bottom: ${space(1)};
+`;
+
+class TableOfContents extends Component {


Let's use a functional component here. The tocbot can be init in useEffect

evanpurkhiser · 2021-09-21T17:05:35Z

docs-ui/stories/core/typography.tsx

+
+type TypeDefinition = {
+  name: string;
+  /** HTML tag (h1, h2, p) used to render type element */


Let's keep it as a normal flag

/** * HTML tag (h1, h2, p) used to render type element */

evanpurkhiser · 2021-09-21T17:06:05Z

docs-ui/stories/core/typography.tsx

+/** Generic component with variable HTML tag name,
+ * to be used with @emotion's as prop.
+ * See: https://emotion.sh/docs/styled#as-prop
+ *   */


comment problems

evanpurkhiser · 2021-09-21T17:07:53Z

static/app/icons/iconCode.tsx

+
+type Props = React.ComponentProps<typeof SvgIcon>;
+
+const IconCode = React.forwardRef(function IconChat(


Suggested change

const IconCode = React.forwardRef(function IconChat(

const IconCode = React.forwardRef(function IconCode(

evanpurkhiser · 2021-09-21T17:08:04Z

static/app/icons/iconCode.tsx

+      <path d="M3.82 12.3a.79.79 0 0 1-.53-.22l-3-3.05a.74.74 0 0 1 0-1.06l3-3a.75.75 0 0 1 1.06 1.01L1.84 8.5l2.51 2.52a.75.75 0 0 1-.53 1.28ZM12.18 12.3a.75.75 0 0 1-.53-1.28l2.51-2.52-2.51-2.52a.75.75 0 1 1 1.06-1.06l3 3.05a.741.741 0 0 1 0 1.06l-3 3.05a.79.79 0 0 1-.53.22ZM5.69 14.25A.75.75 0 0 1 5 13.19l4.62-10a.75.75 0 0 1 1-.36.74.74 0 0 1 .36 1l-4.62 10a.76.76 0 0 1-.67.42Z" />
+    </SvgIcon>
+  );
+});


For some reason I thought we already had this 🤔

Me too! But I looked all over the place, including the list in Storybook, but couldn't find something like this 🤔

evanpurkhiser · 2021-09-21T17:08:19Z

yarn.lock

@@ -14759,6 +14759,11 @@ to-regex@^3.0.1, to-regex@^3.0.2:
    regex-not "^1.0.2"
    safe-regex "^1.1.0"

+tocbot@^4.13.4:


Glad this doesn't come with much baggage

evanpurkhiser · 2021-09-21T17:09:58Z

can we get the icon center with the text?

vuluongj20 · 2021-09-21T18:45:42Z

Apparently the SVG attributes I pasted into IconCode was not vertically aligned (about 1px off) 🤭 Just fixed this now.
However, there is still some small misalignment due to the default baseline settings of the font face. This is not as noticeable. I'll do some further research into font face rendering and look for a way to fix it!

#28680

evanpurkhiser · 2021-09-22T18:50:22Z

docs-ui/components/code.tsx

+      return;
+    }
+
+    /** Remove comments from code */


Avoid using flag comments unless they are multi-line, and are documenting something like a function or module level variable

Can just be

// Remove comments from code

evanpurkhiser · 2021-09-22T18:53:23Z

docs-ui/components/code.tsx

+  const [copied, setCopied] = useState(false);
+
+  function handleCopyCode() {
+    const copyButton = copyButtonRef?.current;


Do you still need this copyButtonRef?

Oh right, no I don't :)

davidenwang · 2021-09-22T19:20:20Z

/gcbrun

billyvg · 2021-09-22T19:23:15Z

/gcbrun

vuluongj20 requested review from a team as code owners September 20, 2021 18:28

evanpurkhiser reviewed Sep 20, 2021

View reviewed changes

Merge branch 'master' into vuluong/storybook-content

9d40e8d

vercel bot deployed to Preview – sentry September 20, 2021 18:36 View deployment

vercel bot deployed to Preview – storybook September 20, 2021 18:36 View deployment

Indentation: tabs --> spaces

d581eee

vercel bot deployed to Preview – sentry September 20, 2021 18:44 View deployment

vercel bot deployed to Preview – storybook September 20, 2021 18:44 View deployment

stylelint fix: change selection syntax

d98d030

To avoid duplicate selector warning from stylelint.

vercel bot deployed to Preview – storybook September 20, 2021 19:02 View deployment

vercel bot deployed to Preview – sentry September 20, 2021 19:02 View deployment

vuluongj20 changed the title ~~Storybook: more global components & content for typography page~~ feat(ui): Storybook – more global components & content for typography page Sep 20, 2021

vuluongj20 requested a review from billyvg September 20, 2021 19:59

billyvg reviewed Sep 20, 2021

View reviewed changes

Rewrite Code as React function + prop types for ColorChips

671caa5

vuluongj20 requested a review from a team as a code owner September 21, 2021 16:49

vercel bot deployed to Preview – sentry September 21, 2021 16:49 View deployment

vercel bot deployed to Preview – storybook September 21, 2021 16:49 View deployment

evanpurkhiser reviewed Sep 21, 2021

View reviewed changes

Merge branch 'master' into vuluong/storybook-content

4c95024

vercel bot deployed to Preview – storybook September 21, 2021 16:55 View deployment

vercel bot deployed to Preview – sentry September 21, 2021 16:55 View deployment

evanpurkhiser reviewed Sep 21, 2021

View reviewed changes

docs-ui/components/sample.tsx Show resolved Hide resolved

evanpurkhiser reviewed Sep 21, 2021

View reviewed changes

Updates based on PR comments

bb05561

#28680

vercel bot deployed to Preview – sentry September 21, 2021 19:03 View deployment

vercel bot deployed to Preview – storybook September 21, 2021 19:03 View deployment

vuluongj20 requested review from evanpurkhiser and billyvg September 21, 2021 19:18

evanpurkhiser reviewed Sep 22, 2021

View reviewed changes

Update code.tsx

bb2996e

vercel bot deployed to Preview – sentry September 22, 2021 19:03 View deployment

vercel bot deployed to Preview – storybook September 22, 2021 19:03 View deployment

evanpurkhiser approved these changes Sep 22, 2021

View reviewed changes

vuluongj20 merged commit 4ac0d63 into master Sep 22, 2021

vuluongj20 deleted the vuluong/storybook-content branch September 22, 2021 19:43

github-actions bot locked and limited conversation to collaborators Oct 8, 2021


		type State = {};

		class Code extends Component<Props, State> {


		type Props = React.ComponentProps<typeof SvgIcon>;

		const IconCode = React.forwardRef(function IconChat(

feat(ui): Storybook – more global components & content for typography page #28680

feat(ui): Storybook – more global components & content for typography page #28680

Conversation

vuluongj20 commented Sep 20, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evanpurkhiser Sep 21, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vuluongj20 Sep 21, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evanpurkhiser commented Sep 21, 2021

vuluongj20 commented Sep 21, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidenwang commented Sep 22, 2021

billyvg commented Sep 22, 2021

vuluongj20 commented Sep 20, 2021 •

edited

Loading

evanpurkhiser Sep 21, 2021 •

edited

Loading

vuluongj20 Sep 21, 2021 •

edited

Loading