Skip to content

docs(#1783): radio and checkbox slot description and example#147

Merged
ArakTaiRoth merged 1 commit into
GovAlta:alphafrom
bdfranck:docs-1783
May 6, 2024
Merged

docs(#1783): radio and checkbox slot description and example#147
ArakTaiRoth merged 1 commit into
GovAlta:alphafrom
bdfranck:docs-1783

Conversation

@bdfranck
Copy link
Copy Markdown
Collaborator

@bdfranck bdfranck commented Apr 22, 2024

This PR for GovAlta/ui-components#1783 has the following documentation changes to radio and checkbox:

  • Adds slot/reactNote type to description property.
  • Adds an example of a component as a description. (Based on this Figma example)
  • Cleans up the other property descriptions and ensures they all end in a period

@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 22, 2024

Deploy Preview for abgov-ui-component-docs ready!

Name Link
🔨 Latest commit 526fd0d
🔍 Latest deploy log https://app.netlify.com/sites/abgov-ui-component-docs/deploys/66325c8954822d000844fad7
😎 Deploy Preview https://deploy-preview-147--abgov-ui-component-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

<goa-form-item label="How would you like to be contacted?">
<goa-checkbox checked="true" name="item" text="Phone">
<div slot="description">
<goa-form-item mt="m" label="Phone number">

This comment was marked as resolved.

Comment thread src/routes/components/Checkbox.tsx
@bdfranck
Copy link
Copy Markdown
Collaborator Author

bdfranck commented Apr 22, 2024

Aha! That answers one of my questions:

image

I'll update the example code.

@bdfranck bdfranck force-pushed the docs-1783 branch 3 times, most recently from 64b1f4c to 2caacad Compare April 22, 2024 23:30
@bdfranck
Copy link
Copy Markdown
Collaborator Author

I tested it and it appears to work. Is there an effective way to test the sample code? 🤔

image

@bdfranck
Copy link
Copy Markdown
Collaborator Author

The Angular version...
image

@bdfranck
Copy link
Copy Markdown
Collaborator Author

bdfranck commented Apr 23, 2024

I noticed the stray React GoAInput component in my Angular example. I've replaced it.
image

@bdfranck bdfranck changed the base branch from main to alpha April 23, 2024 16:03
@bdfranck
Copy link
Copy Markdown
Collaborator Author

I noticed other example files had tab spacing of 2 instead of 4. I updated my example with 2 space tabs.

@bdfranck
Copy link
Copy Markdown
Collaborator Author

Wait- the issue is for the radio, not the checkbox. But we still can use the example in checkbox. I'll add a commit for the radio.

@bdfranck bdfranck changed the title docs(#1783): checkbox slot description and example docs(#1783): radio and checkbox slot description and example Apr 23, 2024
@bdfranck
Copy link
Copy Markdown
Collaborator Author

I've checked how the radio example looks in Angular...
image

...and In React:
image

@lizhuomeng71
Copy link
Copy Markdown
Collaborator

For radio -> angular, there is an react GoAInput tag in angular code, please see screenshot

Screenshot 2024-04-25 140545

@bdfranck
Copy link
Copy Markdown
Collaborator Author

Thanks, @lizhuomeng71! I've updated the React examples with description={} instead of <div slot="description">

@lizhuomeng71
Copy link
Copy Markdown
Collaborator

Hi @bdfranck , sorry, I probably highlighted an wrong item on my previous comment, the point of previous comment is there is an line of react code in angular example.

image

Comment thread src/examples/radio/RadioExamples.tsx Outdated
Comment on lines +23 to +25
<goa-form-item mt="m" label="Input label">
<goa-input name="phoneNumber" (_change)="onChange($event)" />
</goa-form-item>
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@lizhuomeng71 Got it! I've replaced the stray React component with an Angular one in the Angular sample code.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Same thing here for the text based description.

@bdfranck
Copy link
Copy Markdown
Collaborator Author

@lizhuomeng71 Any other questions or concerns? Thanks!

@lizhuomeng71

This comment was marked as resolved.

@lizhuomeng71
Copy link
Copy Markdown
Collaborator

Issue is verified for React and Angular

@lizhuomeng71 lizhuomeng71 marked this pull request as ready for review April 30, 2024 19:50
<goa-checkbox checked="true" name="phone" text="Phone">
<div slot="description">
<goa-form-item mt="m" label="Phone number">
<goa-input name="phoneNumber" (_change)="onChange($event)" />
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The description should only contain plain / stylized text. After talking to @twjeffery I found that it is planned to introduce a reveal slot something in the near future that would be used to contain components like this.

So can you revert the description to something text based?

Comment thread src/examples/radio/RadioExamples.tsx Outdated
Comment on lines +23 to +25
<goa-form-item mt="m" label="Input label">
<goa-input name="phoneNumber" (_change)="onChange($event)" />
</goa-form-item>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Same thing here for the text based description.

@bdfranck
Copy link
Copy Markdown
Collaborator Author

bdfranck commented May 1, 2024

I've changed the example to a common scenario from my ministry: putting a link in the help text. @twjeffery Any concerns with this direction?

Obviously, I don't want to endorse any anti-patterns in our official documentation. 😂

image image

@chrisolsen chrisolsen self-requested a review May 3, 2024 18:37
@ArakTaiRoth ArakTaiRoth merged commit 988a169 into GovAlta:alpha May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Documentation: Update Radio documentation

4 participants