Skip to content

[form-schema] Make rjsf description prop available to components#382

Draft
kriscooke wants to merge 5 commits intodevelopfrom
fix/rjsf-description-prop-passthrough
Draft

[form-schema] Make rjsf description prop available to components#382
kriscooke wants to merge 5 commits intodevelopfrom
fix/rjsf-description-prop-passthrough

Conversation

@kriscooke
Copy link
Contributor

@kriscooke kriscooke commented Mar 5, 2022

Proposed Changes

  • form-schema passes through rjsf's optional description prop for use by form input components in component-library
  • theme styling enabled for description labels
  • description controls added to Storybook

Related to #359

Needs work:

  • [ ] HTML-only case does not yet render description labels (Storybook problem only)
  • styling across the different input components (esp. in the Button theme)
  • double-check accessibility good practice on description labels

@kriscooke kriscooke requested review from junminahn and wenzowski and removed request for junminahn March 5, 2022 03:43
Copy link
Contributor

@wenzowski wenzowski left a comment

Choose a reason for hiding this comment

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

Fantastic start! As far as accessibility goes, let's make sure to update the storybook so the manual axe checks can run? (not a full answer...but something that should at least be checking the font size and color of the new descriptions).

Comment on lines +63 to +65
<Sdescription htmlFor={id} {...styleProps} style={descriptionStyle} className={DESCRIPTION_CLASS}>
{description}
</Sdescription>
Copy link
Contributor

Choose a reason for hiding this comment

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

Each LABEL element is associated with exactly one form control.

This will produce two sibling labels for the same input id. Maybe we could consider wrapping the description into a <label>{title}<span>{description}</span></label> inside the existing label tag?

Alternatively, we could wrap labels around inputs, but this might be a breaking change:
<label>{title}{description}<input /></label>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly what I was thinking in the back of my head, will play around with this...

Each LABEL element is associated with exactly one
form control.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants