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

docs(button,icon): remove MDX files #3636

Conversation

marissahuysentruyt
Copy link
Collaborator

@marissahuysentruyt marissahuysentruyt commented Mar 21, 2025

Description

This is the final installment in child PRs for CSS-986, where we are removing any unnecessary MDX documentation files. This PR migrates any documentation from MDX files to the component's *.stories.js file. No loss of documentation should have occurred, but minor changes, like correcting story names to organize similar stories together or reorganizing content now that there is not an MDX file or fixing sentence-casing, is acceptable. The components affected in this PR include button, icon, and typography. For each component, we have:

  • deleted mdx file in favor of all docs being moved into stories file
  • updated some story names to follow sentence-casing and organize like- stories together
  • in the case of icon, moved any markup related templates into the template.js file (which then makes it consistent with all other components)
  • in the case of typography, we elected to keep the MDX file. I only replaced an internal note with external documentation.

No changeset is required for any of the components since no CSS is changed. VRT is unnecessary.

Jira/Specs

CSS-1051

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

Regression testing

Validate:

  1. The documentation pages for at least two other components are still loading, including:
  • The pages render correctly, are accessible, and are responsive.
  1. 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. (@marissahuysentruyt this PR is a docs-only change)

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. ✨

@marissahuysentruyt marissahuysentruyt self-assigned this Mar 21, 2025
Copy link

changeset-bot bot commented Mar 21, 2025

⚠️ No Changeset found

Latest commit: 2870bc9

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

@marissahuysentruyt marissahuysentruyt added documentation Because documentation is important and shouldn't be broken wip This is a work in progress, don't judge. do not merge A flag for a branch indicating it should not be merged. labels Mar 21, 2025
Copy link
Contributor

github-actions bot commented Mar 21, 2025

File metrics

Summary

Total size: 2.24 MB*

🎉 No changes detected in any packages

* Size is the sum of all main files for packages in the library.
* An ASCII character in UTF-8 is 8 bits or 1 byte.

Copy link
Contributor

github-actions bot commented Mar 21, 2025

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

@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-1051-refactor-button-icon-typography-mdx branch from 34edcc7 to d239ea4 Compare March 26, 2025 00:50
@marissahuysentruyt marissahuysentruyt changed the title docs(button): remove MDX file docs(button,icon,typography): remove MDX files Mar 27, 2025
@marissahuysentruyt marissahuysentruyt added the skip_vrt Add to a PR to skip running VRT (but still pass the action) label Mar 27, 2025
@@ -18,7 +18,7 @@ import {
import { TypographyGroup } from "./typography.test.js";

/**
* Spectrum typography is broken out into several separate components: heading, body, detail, and code. Internationalized typography examples are also shown.
* Spectrum typography is broken out into several separate components: [heading](#heading), [body](#body), [detail](#detail), and [code](#code). [Internationalized typography examples](#internationalized-typography) are also shown.
Copy link
Collaborator Author

@marissahuysentruyt marissahuysentruyt Mar 27, 2025

Choose a reason for hiding this comment

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

Typography is the only component where I have pretty strong feelings that we should keep the MDX file. To me, the organization of the MDX file better suits these docs, as opposed to just alphabetically listing everything. We can absolutely move all of the documentation into the stories file, it's totally possible, and I don't think we're losing any of the docs.

However, I think the current flow of the information on prod regarding the typography component seems more accessible and understandable to a user. It makes more sense to me.

Is that just my personal preference? Thoughts on reverting the typography changes and keeping the MDX file?


// ====== Docs: Internationalization ====== //
/**
* We should note that Hebrew and Arabic are rtl languages somewhere over here.
* In the examples below, Hebrew and Arabic are "RTL" languages, meaning they are read from right to left.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This felt like a todo that we missed and it ended up getting published! 😆 Any suggestions on what we want to say here instead?

@marissahuysentruyt marissahuysentruyt removed wip This is a work in progress, don't judge. do not merge A flag for a branch indicating it should not be merged. labels Mar 27, 2025
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-1051-refactor-button-icon-typography-mdx branch 2 times, most recently from b4afada to 9fe59f2 Compare March 27, 2025 22:34
@marissahuysentruyt marissahuysentruyt marked this pull request as ready for review March 27, 2025 22:42
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-1051-refactor-button-icon-typography-mdx branch from 9fe59f2 to a0f14e9 Compare March 31, 2025 19:29
Copy link
Collaborator

@jawinn jawinn left a comment

Choose a reason for hiding this comment

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

This looks good to me. I just had one minor suggestion for one of the story titles.

@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-1051-refactor-button-icon-typography-mdx branch from a0f14e9 to f982d4c Compare April 3, 2025 16:49
@marissahuysentruyt marissahuysentruyt changed the title docs(button,icon,typography): remove MDX files docs(button,icon): remove MDX files Apr 4, 2025
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-1051-refactor-button-icon-typography-mdx branch from f982d4c to e83e1f4 Compare April 4, 2025 16:55
Copy link
Collaborator

@rise-erpelding rise-erpelding left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks for all of the extra little touches you added in; while we can't control the story order as well on a docs page, I think the organization is better overall in this new version!

- delete mdx file in favor of all docs being in stories file
- update some story names to follow sentence-casing and organize like-
stories together

* docs(icon): remove MDX file

- delete mdx file in favor of all docs being in stories file
- update some story names to follow sentence-casing and organize like-
stories together
- move some markup into template file instead of stories file

* docs(typography): add docs for rtl notes
@marissahuysentruyt marissahuysentruyt force-pushed the marissahuysentruyt/css-1051-refactor-button-icon-typography-mdx branch from e83e1f4 to 2870bc9 Compare April 7, 2025 18:18
@marissahuysentruyt marissahuysentruyt merged commit 0fdad0d into main Apr 7, 2025
12 checks passed
@marissahuysentruyt marissahuysentruyt deleted the marissahuysentruyt/css-1051-refactor-button-icon-typography-mdx branch April 7, 2025 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Because documentation is important and shouldn't be broken ready-for-review skip_vrt Add to a PR to skip running VRT (but still pass the action)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants