Skip to content
This repository has been archived by the owner on Dec 6, 2022. It is now read-only.

Add API docs for src-radio #902

Merged
merged 4 commits into from
Aug 16, 2021
Merged

Add API docs for src-radio #902

merged 4 commits into from
Aug 16, 2021

Conversation

jamie-lynch
Copy link

What is the purpose of this change?

Include the API docs for the radio package in Storybook to make them more visible to users

What does this change?

  • Split the Radio and RadioGroup components into their own files
  • Remove cssOverrides from props as it is inherited from the Props interface already
  • Update the stories title to include the package
  • Update the stories definition to match the styles from Accordion
  • Update component README to point to storybook

Screenshots

image

image

TODO

  • [] Update Zeroheight to point to storybook

@jamie-lynch jamie-lynch requested a review from a team as a code owner August 3, 2021 08:11
@probot-autolabeler probot-autolabeler bot added the Radio Changes to the radio button component label Aug 3, 2021
@jamie-lynch jamie-lynch mentioned this pull request Aug 3, 2021
Merged
SiAdcock
SiAdcock reviewed Aug 3, 2021
View reviewed changes
src/core/components/radio/RadioGroup.tsx

return (
<fieldset
aria-invalid={!!error}
Copy link
Contributor

@SiAdcock SiAdcock Aug 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Any chance we can add a role="radiogroup" while we're here and fix #838?

Copy link
Author

Choose a reason for hiding this comment

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

I get an eslint error when I do that 😢

Non-interactive elements should not be assigned interactive
roles.eslintjsx-a11y/no-noninteractive-element-to-interactive-role

Copy link
Contributor

Choose a reason for hiding this comment

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

This ought to have been fixed in jsx-eslint/eslint-plugin-jsx-a11y#746 so I'm not sure what's happening there. Let's leave it for now, I'll update #838

@SiAdcock SiAdcock mentioned this pull request Aug 6, 2021
Open
SiAdcock
SiAdcock reviewed Aug 6, 2021
View reviewed changes
src/core/components/radio/radio.stories.tsx
@@ -1,11 +1,216 @@
import { Radio } from './index';
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: filename ought to be Radio.stories.tsx

Copy link
Author

@jamie-lynch jamie-lynch Aug 6, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Ah yes. I thought I'd done this but it seems that it didn't recognise radio -> Radio as a change so had to hack it slightly.

@sndrs sndrs changed the title Add Radio API docs to storybook Add API docs for src-radio Aug 9, 2021
@SiAdcock SiAdcock force-pushed the radio-docs branch 2 times, most recently from 710bd07 to 705b302 Compare August 16, 2021 11:11
SiAdcock and others added 4 commits August 16, 2021 12:13
@SiAdcock
extend story stub with default args and arg types
c6fe907
@SiAdcock
remove api docs from readme
aaf9f92 
Co-authored-by: Jamie Lynch <jamie.lynch@theguardian.com>
@SiAdcock
extract radio components into separate modules
c808719 
Co-authored-by: Jamie Lynch <jamie.lynch@theguardian.com>
@SiAdcock
refactor stories into component-specific stories files
884c2f9 
Co-authored-by: Jamie Lynch <jamie.lynch@theguardian.com>
sndrs
sndrs reviewed Aug 16, 2021
View reviewed changes
src/@types/storybook-emotion-10-fixes.ts
@@ -13,9 +13,10 @@ export type Parameters = {
[key: string]: any;
};

export type Story = {
(arg0: any): JSX.Element;
export type Story<T = Args> = {
Copy link
Member

Choose a reason for hiding this comment

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

is this needed now?

sndrs
sndrs approved these changes Aug 16, 2021
View reviewed changes
Copy link
Member

@sndrs sndrs left a comment

Choose a reason for hiding this comment

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

mob reviewed by @guardian/client-side-infra

@jamie-lynch jamie-lynch merged commit 8f60b7c into main Aug 16, 2021
@jamie-lynch jamie-lynch deleted the radio-docs branch August 16, 2021 13:33
sndrs pushed a commit that referenced this pull request Aug 17, 2021
@SiAdcock @sndrs
Add API docs for src-radio (#902)
7341986 
* extend story stub with default args and arg types

* remove api docs from readme

Co-authored-by: Jamie Lynch <jamie.lynch@theguardian.com>

* extract radio components into separate modules

Co-authored-by: Jamie Lynch <jamie.lynch@theguardian.com>

* refactor stories into component-specific stories files

Co-authored-by: Jamie Lynch <jamie.lynch@theguardian.com>

Co-authored-by: Simon Adcock <simonadcock2@gmail.com>
Co-authored-by: Jamie Lynch <jamie.lynch@theguardian.com>
@mxdvl mxdvl mentioned this pull request Oct 6, 2021
Merged
10 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@SiAdcock SiAdcock SiAdcock left review comments

@sndrs sndrs sndrs approved these changes

Assignees
No one assigned
Labels
Radio Changes to the radio button component
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jamie-lynch @sndrs @SiAdcock