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] Revise Joy UI "Radio" page #35893

Merged
merged 30 commits into from
Mar 30, 2023
Merged

Conversation

DevinCLane
Copy link
Contributor

@DevinCLane DevinCLane commented Jan 20, 2023

part of #33998

WIP

Started to work on the Title section as well as the introduction. Mainly working on grammar, style, consistency.

Feel free to let me know any comments you might have.

Thanks!

@mui-bot
Copy link

mui-bot commented Jan 20, 2023

Netlify deploy preview

Bundle size report

No bundle size changes

Generated by 🚫 dangerJS against c893989

@danilo-leal danilo-leal changed the title Joy radio [docs] Revise Joy UI "Radio" page Jan 23, 2023
@danilo-leal danilo-leal added docs Improvements or additions to the documentation package: joy-ui Specific to @mui/joy component: radio This is the name of the generic UI component, not the React module! labels Jan 23, 2023
@danilo-leal danilo-leal marked this pull request as draft January 23, 2023 03:40
@samuelsycamore
Copy link
Member

Thanks for taking this on @DevinCLane !! Holler at me when you're ready for a review!

@DevinCLane
Copy link
Contributor Author

Hey @samuelsycamore, we're ready for a review.

"Anatomy" section could use a look.

I used the Radio Group page from Material UI to match for consistency. Also read through your work on Checkbox (#35817) for inspiration.

Thanks!

@DevinCLane DevinCLane marked this pull request as ready for review January 25, 2023 20:16
@samuelsycamore
Copy link
Member

This is a great start! The content you've added is definitely an improvement over the existing page.

Before I jump into a line-by-line reading, let's talk about the big picture. Peep the h2 and h3 headers listed in the right-side table of contents in the Alert component doc, for example—this one serves as a good template for how all of these pages should be structured. At a minimum we should see h2s for Introduction, Basics, Customization, and Anatomy (in that order). The Radio page also includes sections for Common examples and Accessibility, which are h2s that are frequently (but not always) found on these pages.

In the next draft, try to match this structure as closely as possible. Don't be afraid to do some heavy rearranging and rewriting as needed.

Finally, I would caution against leaning too heavily on the Material UI docs for reference—it can be helpful in some cases, but these docs have never seen the hand of an editor before, so many are in various states of disorder/disarray by my standards. 😅 The best reference would be the other Joy UI doc pages that have already been revised.

@DevinCLane
Copy link
Contributor Author

Got it, will rearrange and rewrite as needed to fit that structure. And I will divest from the Material UI as a style guide and look more to the other edited Joy UI docs to match the voice.

@DevinCLane
Copy link
Contributor Author

I rearranged things to better match the structure--let me know if this works or if I've missed anything!

@DevinCLane
Copy link
Contributor Author

@samuelsycamore following up here to see if the updated structure works!

@samuelsycamore
Copy link
Member

Sorry for the delay on my end @DevinCLane ! Structure is looking good. In the next pass, try to make all of the component names consistent. Our engineers use various style patterns:

  • Form Control
  • FormControl
  • FormControl
  • form control
  • etc.

Only the first one is correct.

@DevinCLane
Copy link
Contributor Author

Made a pass at consistent component formatting.

@samuelsycamore I imagine you might have some thoughts on when a component such as Radio needs to be capitalized "Radio" vs when we are talking about "a radio button". Did my best to stay consistent here: when it felt like we were talking about the concept of a general radio button, left uncapitalized; when it seemed we were discussing the Radio component, capitalized it.

@samuelsycamore
Copy link
Member

I apologize @DevinCLane, I don't think I ever shared the Style Guide with you! I definitely have some thoughts on this topic. 😁 You can find those in the Style Conventions for MUI Components doc.

@DevinCLane
Copy link
Contributor Author

@samuelsycamore: read through the style guide and made some updates. Couldn't seem to find a mention of how to style props and CSS properties, I figured that was in line with referring to code so I left them with backticks.

Let me know what you think!

Copy link
Member

@samuelsycamore samuelsycamore left a comment

Choose a reason for hiding this comment

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

Thanks @DevinCLane ! You picked a doozy of a doc to tackle! Let me know if you have any questions about my edits here.

docs/data/joy/components/radio/radio.md Outdated Show resolved Hide resolved
docs/data/joy/components/radio/radio.md Outdated Show resolved Hide resolved
docs/data/joy/components/radio/radio.md Outdated Show resolved Hide resolved
docs/data/joy/components/radio/radio.md Outdated Show resolved Hide resolved
docs/data/joy/components/radio/radio.md Outdated Show resolved Hide resolved
docs/data/joy/components/radio/radio.md Outdated Show resolved Hide resolved
docs/data/joy/components/radio/radio.md Outdated Show resolved Hide resolved
docs/data/joy/components/radio/radio.md Outdated Show resolved Hide resolved
docs/data/joy/components/radio/radio.md Outdated Show resolved Hide resolved
docs/data/joy/components/radio/radio.md Outdated Show resolved Hide resolved
@DevinCLane
Copy link
Contributor Author

Great, thanks for the edits. Will take a look, work on implementing, and let you know if I have any questions.

DevinCLane and others added 4 commits February 10, 2023 11:39
Co-authored-by: Sam Sycamore <71297412+samuelsycamore@users.noreply.github.com>
Signed-off-by: Devin Lane <devin.c.lane@gmail.com>
@DevinCLane
Copy link
Contributor Author

DevinCLane commented Feb 10, 2023

@samuelsycamore

  • Applied your line edits
  • Reordered variants, breaking them into their own sections, added demos

The preexisting demos were still written in .js and therefore don't have their .tsx.preview files to match the format you have in the updated Joy Badge docs, for example. I imagine we might want to change these to match?

Looking forward to the comments, cheers!

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 18, 2023
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 20, 2023
Copy link
Member

@siriwatknp siriwatknp 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, thanks for the improvements!

Copy link
Member

@samuelsycamore samuelsycamore left a comment

Choose a reason for hiding this comment

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

Great job @DevinCLane ! I just made a few tiny copy edits to wrap things up here. Thanks for your patience!

@samuelsycamore samuelsycamore merged commit 0687565 into mui:master Mar 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: radio This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation package: joy-ui Specific to @mui/joy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants