feat: ResourceListDetails doc block #3129

marissahuysentruyt · 2024-09-18T14:25:02Z

Description

In the effort to bring as much of the docs site into Storybook, this PR creates a new <ResourceListDetails /> doc block component. It should replicate this resource section of the docs site:

<ResourceListDetails /> should render each of the resource link cards (<ResourceLinkContent />) if the link exists. To do so, we are pulling data from each component's package.json. A few things should happen with that data:

we can get the package name, which corresponds to the package on NPM
using that package name, we can programmatically create links to the component's package on NPM and its place in the repo (navigating to the component's README)

There was also a need for a sub-component, <ResourceLinkContent />, to hold the individual resource's content (the SVG icon, the "title" and "subtitle"). There are several styled components as well, mainly used as containers/wrappers, that made up the basis of the styles for the two main components.

Component package.json updates

Adding a spectrum object to each component's package.json (with a guidelinesLink and rootClass keys) gives us the ability to grab the link to the Spectrum Guidelines site. For the majority of components, this single addition is sufficient.
a. Some package.json files can generate multiple components, such as progress bar and meter. Those components have multiple objects within spectrum, to hold the data for the individual components.
In cases where Spectrum Guidelines do not exist for a component (like accordion or dial), if documentation could be found on the Spectrum Contributions site, the guidelinesLink was pointing to that site instead.
For cases where no guidelines could be found on either of the sites mentioned above, the spectrum array was not added to the package.json. This allows us to just not render that specific guidelines card.

Uncovered issues

The switch and progress bar do not render some of the doc blocks. Neither renders the ComponentDetails or this new ResourceListDetails block. Although both render TaggedReleases, I believe there's issues with the data coming back from NPM for those two components. There is one tag found, at 0.1.0, and that link goes to a deprecated NPM package called undefined.js.

Resolved!

Switch and progress bar were missing some parameters for the package metadata: d88aeba

More details and screenshots regarding this issue can be found in the thread of this message.

Jira/Specs

CSS-772

How and where has this been tested?

Please tag yourself on the tests you've marked complete to confirm the tests have been run by someone other than the author.

Validation steps

It is necessary to be on the VPN in order to verify and access some of the Spectrum Contributions links.

In your code editor:

With the project running, open the .storybook/blocks/ComponentDetails.jsx file
In the ResourceListDetails component, change the linkType prop to any value you'd like. (in this example, I used "sparkles")

The project should render, and if you look in the browser, you should see a warning message, similar to the one below, catching the invalid linkType value:

Revert that change and ensure the project renders as normal.
Visit any component docs page (like http://localhost:8080/?path=/docs/components-help-text--docs)
Find that component's package.json (/components/your-component/package.json)
At the bottom of the package.json, you should see either the spectrum key. Test 3 scenarios:

If the key had a value for componentName, change the value to an empty string.
- Back in the browser, the guidelines resource card should not render.

If the key had a value, change it to something else.
- Back in the browser, the guidelines resource card should render. When you click on that card, you should navigate to a URL path with your new value.
If the key was an empty string, set the value to something new.
- Back in the browser, the guidelines resource card should render. When you click on that card, you should navigate to a URL path with your new value.

Verify that form and meter are defined as separate objects in the spectrum array in the field-label and progress-bar package.json files respectively.
Check that the cards flow responsively when the window resizes.

Things to note

Some components don't have a 1:1 match when comparing their name to the title of their Spectrum Guidelines page (i.e. radio => radio group; search => search field). Those were confirmed as the correct link to navigate to.
For some components using the Spectrum Contributions site as guidelines, some educated guesses were made. Some components are linked to their default variant (i.e. dialog => standard dialog; pagination => pagination list). Others are linked to a similar name or have a proposed name change (i.e well => wells; stepper => number field).
There are 3 components in components/ that are not included in this work: page, site, commons.
If you find that the page running locally doesn't render or see a similar Quota exceeded. Failed to execute error in the browser console:

you have to first clear your local storage, then clear your browser cache.

Regression testing

Validate:

The documentation pages for at least two other components are still loading, including:

The pages render correctly, are accessible, and are responsive.

If components have been modified, VRTs have been run on this branch:

VRTs have been run and looked at.
Any VRT changes have been accepted (by reviewer and/or PR author), or there are no changes.

Screenshots

To-do list

I have read the contribution guidelines.
I have updated relevant storybook stories and templates.
I have tested these changes in Windows High Contrast mode.
If my change impacts other components, I have tested to make sure they don't break.
If my change impacts documentation, I have updated the documentation accordingly.
✨ This pull request is ready to merge. ✨

changeset-bot · 2024-09-18T14:25:06Z

⚠️ No Changeset found

Latest commit: 47b110b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

github-actions · 2024-09-18T14:30:09Z

🚀 Deployed on https://pr-3129--spectrum-css.netlify.app

github-actions · 2024-09-18T14:34:58Z

File metrics

Summary

Total size: 4.31 MB^*

🎉 No changes detected in any packages

* Size determined by adding together the size of the main file for all packages in the library.
* Results are not gzipped or minified.
* An ASCII character in UTF-8 is 8 bits or 1 byte.

.storybook/blocks/ComponentDetails.jsx

components/alertdialog/package.json

components/assetcard/package.json

marissahuysentruyt · 2024-09-19T17:48:15Z

components/colorhandle/package.json

@@ -55,5 +55,6 @@
 	],
 	"publishConfig": {
 		"access": "public"
-	}
+	},
+	"componentGuidelinesName": ""


On the docs site, the color handle guidelines link is pointing to the color area guidelines.

Do we want to replicate that link for color handle? Currently, color handle isn't rendering a guidelines card.

components/infieldbutton/package.json

components/pickerbutton/package.json

.storybook/blocks/ComponentDetails.jsx

jawinn

This is getting close! The review comments are for the current technique that is comparing the story title for the guidelines link. Some of those may be moot if we go this direction:

I had an idea to simplify the comparison between the package.json rootClass and which story is being rendered, without the need to compare with the title and use reformatTitle.

You should be able to compare the spectrumData[i]?.rootClass against the default arg value of rootClass within the stories file. This can be found in const rootClassArg = storyMeta?.csfFile?.args?.rootClass;:

Part of the struggle with using this, as we had chatted about before, is the Meter component, which is housed with progress bar and also uses the same rootClass as progress bar in its templates. It seems like changing its rootClass to be its own would be a good idea, to make it consistent with the others. I took a stab at adjusting that here:
#3231

I also want to check with the rest of the team to see if there's anything else that the change would negatively affect. It seems to make sense, also because spectrum-Meter is defined as the expected rootClass in the package.json

.storybook/blocks/ComponentDetails.jsx

components/colorloupe/package.json

The changes I requested have been addressed, thanks; I'm dismissing the requested changes so they are not blocking.

.storybook/blocks/Typography.jsx

rise-erpelding

Hi! I'll keep looking through components to make sure everything looks good, but I'm leaving a review with my questions on the deprecated components!

rise-erpelding · 2024-10-16T14:54:14Z

.storybook/blocks/ComponentDetails.jsx

-				</DList>
+						}
+					</DList>
+					<ResourceListDetails packageName={packageName} spectrumData={spectrumData} rootClassName={rootClassName}/>


One very minor thing that I'm noticing is that the GitHub links for the deprecated components don't work as intended. If it's not worthwhile even worrying about that and that was a conversation that already happened somewhere, that's fine!

If not, there are probably several ways we could handle this, and at this point you would probably have the best judgment on which to use, but some thoughts that come to my mind are:

Only display the npm link for the deprecated components - either pass down isDeprecated to ResourceListDetails (I don't love this idea), or maybe only conditionally render the GitHub ResourceLinkContent if spectrumData.length !== 0?

Don't render ResourceListDetails for the deprecated components at all? So either only render ResourceListDetails if isDeprecated is false, or return early if there's nothing in the spectrumData array (if (!packageName || spectrumData.length === 0) return;)?

What do you think?

Going to note now that I've gone through the rest of the components that Action menu has no spectrumData (there's no spectrum key in the package.json), is that deliberate? If so that would have some unintended side effects with these approaches I just mentioned that involve checking the length of spectrumData

Correct- it was deliberate to not have a spectrum key for action menu.

.storybook/blocks/ComponentDetails.jsx

marissahuysentruyt · 2024-10-16T17:39:35Z

.storybook/blocks/ComponentDetails.jsx

+				href={status ?
+					`https://github.com/adobe/spectrum-css/tree/main/.storybook/deprecated/${packageName.split('/').pop()}`
+					: `https://github.com/adobe/spectrum-css/tree/main/components/${packageName.split('/').pop()}`}/>


This is my attempt at trying to replicate the approach we already had in place for the GitHub links. I'm passing the isDeprecated variable, set in ComponentDetails down to ResourceListDetails.

To me, that felt the most simple. When deprecating a component, we already have to go into the story file and add status: { type: "deprecated" } to the parameters. I thought about adding some sort of "repoLink" or "deprecatedRepoLink" to the spectrum data in the deprecated components' package.json's, but that felt like an extra step we didn't really need to add to the deprecation process.

Thoughts?

rise-erpelding

I think we're just about there!

rise-erpelding · 2024-10-16T18:06:47Z

.storybook/blocks/ComponentDetails.jsx

+	const packageJson = data;
+
+	return (
+		<ResourceSection skipBorder={true} className="sb-unstyled">


I just noticed that skipBorder={true} was here, do we still need it for anything? It doesn't look like we're using this prop as far as I can see.

rise-erpelding · 2024-10-16T18:23:23Z

.storybook/blocks/ComponentDetails.jsx

@@ -356,7 +356,7 @@ export const ResourceLinkContent = ({ heading, alt, logo, href }) => {
 * @param {string} rootClassName - a component's default rootClass arg
 * @returns {string}
 */
-export const ResourceListDetails = ({ packageName, spectrumData = [], rootClassName }) => {
+export const ResourceListDetails = ({ packageName, spectrumData = [], rootClassName, status }) => {


I can't think of a way to avoid passing another arg here either, now that I know there are other components that don't have spectrumData... so I think this approach works fine! Any objection to changing the name from status to isDeprecated? Since status is a boolean that is only true for deprecated components?

sure! Just pushed up that change, along with removing that skipBorder. 5376ff8

- components that have a valid page on the spectrum guidelines site have a componentGuidelinesName key now. - components that do not have a valid page on the spectrum guidelines site have a componentBetaName key that will point to the spectrum-contributions site. - for components that do not any guidelines, setting the componentGuidelinesName key to an empty string in the package.json will make sure the guidelines resource card doesn't render.

The ResourceListDetails fetches data from each storybook component's package.json, specifically utilizing the package name and optional componentGuidelinesName. It then creates links to the spectrum guidelines, github, and npm sites. If a component doesn't have a spectrum guidelines page, it uses the optional componentBetaName and the spectrum-contributions site instead. - creates new `styled` components related to the resource section, any element wrappers and links - creates ResourceListDetails doc block - creates ResourceListContent component (which are the individual resource cards) - adds some helper functions to generate SVGs corresponding to the links, switch statements, mapping text, etc.

this applies to form and meter. Form should no longer render a guidelines link card, and meter should navigate to its own guidelines page instead of progress bar's.

- update spacing for if() statement - render ResourceListDetails in ComponentDetails - console.warn instead of throw an error - hard codes styles for cards

- extracts SVG code to individual jsx files - imports those SVG jsx files to use a components - replaces/refactors svg functions to render new svg components

- creates a `spectrum` key in each component's package.json to use in ComponentDetails - `spectrum` array holds metadata for components and/or nested components, like the componentName, the guidelineLink, and rootClass - eventually we may be able to add designLink to add Figma links to this block

- reorganizes file so ResourceLinkContent and ResourceListDetails are closer to CopmonentDetails, and all fetching/processing helper functions are together - addresses missing field label guidelines link - removes some hardcoded nestedComponent code in favor for a for loop - indentation fixes

- removed componentName from field label and progress bar package.jsons - created docsPage parameter for form in the storyMeta - refactored the nested component for loop to check if the guidelines are defined, and if docsPage is undefined. If both of those check out, render the guidelines link. Form is the only component that needed this, since it's the only nested component that doesn't have a guidelines page while its parent (field label) does.

To better capture nested components, as well as components that may not have a guidelines page, this refactor double checks that the hasDocsPage prop is undefined, as well as that the spectrum.rootClass value includes a transformed component title. Adding the additional check of the rootClass helped ensure the proper guidelines link was rendering for nested components, as well as breaking when the condition was met.

- instead of comparing a story's title (because meter didn't have a unique rootClass), compare the spectrum.rootClass to the meta.args.rootClass. This is possible now because we have updated the default args for meter to be spectrum-Meter. - removes usage or reformatTitle and hasDocsPage functions and props - adds more thorough JSDoc comments

- white space removal

- change status prop name to isDeprecated - remove unnecessary skipBorder prop

marissahuysentruyt added the wip This is a work in progress, don't judge. label Sep 18, 2024

marissahuysentruyt force-pushed the marissahuysentruyt/css-772-link-doc-blocks branch 5 times, most recently from 82e4159 to 698a73a Compare September 19, 2024 16:46