Port over draft components #718

camertron · 2024-01-29T22:46:58Z

See #710 for the migration of deprecated components.

This PR migrates docs for draft React components from the old primer.style/react site to the new docsite. This was not a straightforward migration, but was significantly easier than deprecated components, since all draft components are available in components.json.

However, draft components presented the following challenges:

Additional documentation is present in bespoke .md files.
Although all draft components are listed in components.json, they are often listed with inconsistent IDs. For example, dialog's ID is drafts_dialog, but select panel's is selectpanel_v2. Note that the alpha version of select panel has an ID of select_panel 🤷
Some components listed under "Drafts" on primer.style/react (the old docsite) are actually deprecated in components.json.

The following draft components have been migrated in this PR:

DataTable: This one was actually already available in the new docsite.
Dialog v2: Available at /components/dialog/react/draft
Hidden: Added a new overview page and the draft component page.
InlineAutocomplete: Deprecated.
MarkdownEditor: Deprecated.
MarkdownViewer: Deprecated
PageHeader: Dedicated page available at /components/page-header/react/draft
SelectPanel: Dedicated page available at /components/selectpanel/react/draft
Tooltip: Dedicated page available at /components/tooltip/react/draft

NOTE: I discovered you can export a special Layout component from .mdx files, which greatly simplified several of the already-migrated deprecated components.

mperrotti · 2024-01-31T19:11:25Z

Additional documentation is present in bespoke .md files.

I think we're trying to to move away from this and rely mostly on Storybook and .docs.json files. cc @broccolini @joshblack @pksjce @siddharthkp

Although all draft components are listed in components.json, they are often listed with inconsistent IDs
Some components listed under "Drafts" on primer.style/react (the old docsite) are actually deprecated in components.json

Oof - this is no good. We should correct these inconsistencies. @camertron - are you opening a PR to handle this in primer/react? Or should I?

mperrotti

Reviewing this PR has given me a scary look into the current state of our component docs. I did my best to make helpful suggestions, but I don't think I fully understand how things work under the hood. Please share any documentation, Looms, etc that might help me better understand how everything works.

content/components/dialog/react/draft.mdx

mperrotti · 2024-01-31T19:37:07Z

src/components/react-props-table.tsx

@@ -0,0 +1,78 @@
+import React from 'react'


This table doesn't look right. It's not consistent with https://github.com/primer/react/blob/main/docs/src/props-table.js or https://github.com/primer/design/blob/main/src/components/props-table.jsx which have a "type" column.

Can we just get rid of this table and use https://github.com/primer/design/blob/main/src/components/props-table.jsx ?

I actually extracted this from react-component-layout.tsx, meaning it's the props table we've been using on all our React component pages for a while now. That said, I'm happy to look at replacing it for consistency.

mperrotti · 2024-01-31T19:39:31Z

content/components/dialog/react/draft.mdx

+
+<ReactPropsTable props={props.data.reactComponent.props} />
+
+### DialogHeaderProps


Am I correct in assuming that we have to manually render these tables in Markdown because we don't have a way to document these types in the current shape of the *.docs.json objects?

A good follow-up issue could be to solve this problem so we don't have to manually render stuff like this.

I'm not sure, I just copy/pasted this from primer/react. The info for all these props exists in components.json, so I think we can remove this and let the layout render the props table(s) instead.

mperrotti · 2024-01-31T19:45:52Z

content/components/dialog/react/draft.mdx

+### useConfirm hook
+
+An alternate way to use `ConfirmationDialog` is with the `useConfirm()` hook. It takes no parameters and returns an `async` function, `confirm`. When `confirm` is called (see ConfirmOptions below for its argument), it shows the confirmation dialog. When the dialog is dismissed, a promise is resolved with `true` or `false` depending on whether or not the confirm button was used.
+
+### Example (with `useConfirm` hook)
+
+```javascript live noinline
+function ShorthandExample2() {
+  const confirm = useConfirm()
+  const buttonClick = React.useCallback(
+    async function (e) {
+      if (await confirm({title: 'Are you sure?', content: 'You must confirm this action to continue.'})) {
+        e.target.textContent = 'Confirmed!'
+      }
+    },
+    [confirm],
+  )
+  return (
+    <>
+      <Button onClick={buttonClick}>Confirmable action</Button>
+    </>
+  )
+}
+render(<ShorthandExample2 />)
+```


Can we move this to the useConfirm() hook docs and just link there?

Hmm yeah that's a good idea.

mperrotti · 2024-01-31T20:23:58Z

content/deprecated-components/markdown-viewer.mdx

@@ -0,0 +1,22 @@
+---
+title: Markdown viewer


This has been moved to Primer Recipes and doesn't need to ported over https://ui.githubapp.com/storybook/?path=/story/recipes-markdownviewer--default

Is it possible someone is using this component in dotcom and needs to reference the deprecated docs?

Not seeing it in Primer Query

mperrotti · 2024-01-31T20:24:16Z

content/deprecated-components/markdown-viewer/react/deprecated.mdx

+  </ReactComponentPage>
+}
+
+The `MarkdownViewer` displays rendered Markdown and handles interaction (link clicking and checkbox checking/unchecking) with that content.


I think we can just link to Primer Recipes https://ui.githubapp.com/storybook/?path=/story/recipes-markdownviewer--default

Is it possible someone is using this component in dotcom and needs to reference the deprecated docs?

Not seeing it in Primer Query

mperrotti · 2024-01-31T20:25:28Z

gatsby-node.esm.js

@@ -215,6 +215,20 @@ async function sourcePrimerRailsData({actions, createNodeId, createContentDigest
  }
 }

+const reactIdMap = {


So do we have to manually handle the data differences between the draft component and the "old" component?

Sort of. This map just helps unify all the components under one name so we can render a status dropdown.

mperrotti · 2024-01-31T20:28:16Z

src/@primer/gatsby-theme-doctocat/nav.yml

+    - title: Hidden
+      url: /components/hidden


I think we should remove internal components from the sidebar until we have a plan for how we want to handle documentation for internal components.

Suggested change

- title: Hidden

url: /components/hidden

mperrotti · 2024-01-31T20:28:53Z

src/@primer/gatsby-theme-doctocat/nav.yml

+    - title: Inline autocomplete
+      url: /deprecated-components/inline-autocomplete
+    - title: Markdown editor
+      url: /deprecated-components/markdown-editor
+    - title: Markdown viewer
+      url: /deprecated-components/markdown-viewer


These have been moved to Primer Recipes and don't need to be maintained in the Primer core docs.

Suggested change

- title: Inline autocomplete

url: /deprecated-components/inline-autocomplete

- title: Markdown editor

url: /deprecated-components/markdown-editor

- title: Markdown viewer

url: /deprecated-components/markdown-viewer

mperrotti · 2024-01-31T20:40:36Z

After giving it more thought, I think we need to have a different react ID for the "draft" version of an existing component. They're often a full rebuild of the original, and have different component APIs.

camertron · 2024-01-31T21:43:49Z

After giving it more thought, I think we need to have a different react ID for the "draft" version of an existing component. They're often a full rebuild of the original, and have different component APIs.

This is an interesting point. As things stand now, components can only have one status, which includes draft, deprecated, etc. The thing is, we could have three versions of the same component, two of which are deprecated, but there's no way of expressing that in the current system. I think we should have two fields, one for the component's version and one for its status.

camertron · 2024-01-31T21:49:20Z

I think we're trying to to move away from this and rely mostly on Storybook and .docs.json files.

For sure. Most of those bespoke .md files actually import the associated .docs.json file and use it to render component props. However they also contain documentation that is separate from what's contained in components.json (the amalgamation of all the .docs.json files, built by CI), and I think in some cases it's important to express those longer-form docs. We could add them to components.json, but JSON is a pretty bad medium for long-form text, especially markdown.

Many of your review feedback here is specific to the contents of these bespoke .md files. For the most part, I copied these over wholesale from primer/react. My goal is simply to make them look decent so we can unpublish and archive the old docsite. Since we're focusing on a revamped docs experience this quarter, I don't think it makes sense to put too much more time and effort into getting them 100% correct, especially considering they're drafts anyway.

mperrotti · 2024-02-01T16:29:17Z

Many of your review feedback here is specific to the contents of these bespoke .md files.

Feel free to ignore that feedback so we don't blow up the scope of this PR too much. We can take care of it in a follow-up.

mperrotti · 2024-02-06T17:24:16Z

src/components/react-component-page.tsx

+                    },
+                  }}
+                >
+                  <StatusMenu currentStatus={status} statuses={statuses} parentPath={`${data.sitePage.path}/react`} />


When I navigate to /latest, will I get the newest version (usually "draft"), or will I get the version that's higher precedence in the statusOrder array?

mperrotti · 2024-02-06T17:24:21Z

In your first Rewatch video that explains our documentation infrastructure (around the 10-minute mark), you say that we shouldn't be adding content in a {component-name}/react/{status}.mdx file anymore. Then in your second Rewatch video (around the 12-minute mark) you mention wanting to clean up these bespoke pages next quarter. Have we started strategizing on how we'll do this work?

Port over draft components

56fd2be

camertron temporarily deployed to github-pages January 29, 2024 22:51 — with GitHub Actions Inactive

Fix dialog props

629d004

camertron deployed to github-pages January 29, 2024 23:06 — with GitHub Actions View deployment

camertron marked this pull request as ready for review January 29, 2024 23:14

camertron requested a review from a team as a code owner January 29, 2024 23:14

camertron requested a review from mperrotti January 30, 2024 05:06

mperrotti reviewed Jan 31, 2024

View reviewed changes

mperrotti reviewed Feb 6, 2024

View reviewed changes

camertron closed this Oct 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port over draft components #718

Port over draft components #718

camertron commented Jan 29, 2024 •

edited

Loading

mperrotti commented Jan 31, 2024

mperrotti left a comment

mperrotti Jan 31, 2024

camertron Jan 31, 2024

mperrotti Jan 31, 2024

camertron Jan 31, 2024 •

edited

Loading

mperrotti Jan 31, 2024

camertron Jan 31, 2024

mperrotti Jan 31, 2024

camertron Jan 31, 2024

mperrotti Feb 1, 2024

mperrotti Jan 31, 2024

camertron Jan 31, 2024

mperrotti Feb 1, 2024

mperrotti Jan 31, 2024

camertron Jan 31, 2024

mperrotti Jan 31, 2024

mperrotti Jan 31, 2024

mperrotti commented Jan 31, 2024

camertron commented Jan 31, 2024

camertron commented Jan 31, 2024

mperrotti commented Feb 1, 2024

mperrotti Feb 6, 2024

mperrotti commented Feb 6, 2024


		<ReactPropsTable props={props.data.reactComponent.props} />

		### DialogHeaderProps

Port over draft components #718

Port over draft components #718

Conversation

camertron commented Jan 29, 2024 • edited Loading

mperrotti commented Jan 31, 2024

mperrotti left a 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

camertron Jan 31, 2024 • 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

mperrotti commented Jan 31, 2024

camertron commented Jan 31, 2024

camertron commented Jan 31, 2024

mperrotti commented Feb 1, 2024

Choose a reason for hiding this comment

mperrotti commented Feb 6, 2024

camertron commented Jan 29, 2024 •

edited

Loading

camertron Jan 31, 2024 •

edited

Loading