RocketCommunicationsInc / astro

Astro UXDS is a collection of guidelines, patterns and components for designing space-based user interface applications.
https://astrouxds.com
Other
115 stars 25 forks source link

Km 4221 checkbox help-text/error-text add slot #878

Closed kiley-mitti closed 2 years ago

kiley-mitti commented 2 years ago

Brief Description

This is the finalized PR for adding help-text/error-text slots for each element that has help text/error-text.

During the process of developing a reasonable way to do this, it was suggested that perhaps error-text should have a slot as well, so we added that to this draft as well.

A help-text and error-text slot have been added to all components that previously only had help-text and/or error-text props.

Big thanks to @FMorrison87 for all of her help with logic and then propagation!

JIRA Link

ASTRO-4221

Related Issue

Now that we are no longer using FormFieldMessage, the component can likely be deleted, but the CSS will need to stay.

General Notes

In keeping with the way help-text and error-text prop were rendered, the render order hierarchy is now this: Error Slot -> Error Prop -> Help Slot -> Help Prop -> nothing. IE: Error will always override help, as it does in current implementation.

This is actually achieved through a series of logic which renders the surrounding part for error-text or help-text hidden or not hidden.

If there is no error slot present it will attempt to fall back to error-text prop, if there is no help slot present it will attempt to fall back to help-text prop.

I am reasonably certain that, while this change extends to several components, it is not a breaking change and only adds features. However, I'm not 100% sure how it will effect other astro packages such as the react package, which is why I haven't deleted the FormFieldMessage component entirely yet.

Motivation and Context

Astro design for help text on inputs shows an ability to add extra html elements to help-text areas, like links. This can't be achieved with a help-text prop alone. We need a slot to give a developer extra control over that area.

Issues and Limitations

Several solutions that utilized the current utility Functional Component: FormFieldMessage were attempted here, but all of them fell short. Usually this had to do with rendering new slot content when the slot was updated dynamically.

In the end, the simpler, more succinct way to code this was to remove FormFieldMessage from and place the logic/styles/html for rendering help and error messages into the component itself.

This means that when we reproduced this functionality in other components we had to copy some limited code to each one. However, the help-text/error-text styling is still controlled by the FormFieldMessage css, so stylistic changes to help/error can still be made in one spot.

Types of changes

Checklist

changeset-bot[bot] commented 2 years ago

🦋 Changeset detected

Latest commit: d1b5828933f85e79ab3a3d3c94260047c150f1f4

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

netlify[bot] commented 2 years ago

Deploy Preview for astro-preview ready!

Name Link
Latest commit d1b5828933f85e79ab3a3d3c94260047c150f1f4
Latest deploy log https://app.netlify.com/sites/astro-preview/deploys/6369586319180900085afeb0
Deploy Preview https://deploy-preview-878--astro-preview.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 settings.

markacianfrani commented 2 years ago

minor nit: individual checkboxes dont have error text bc they dont have invalid states. the checkbox group is responsible for displaying those. but assuming this is the implementation that the rest of the form components will use, this looks good! If everything shares the same styles, I dont think the duplication will be an issue. It's very unlikely that we'll change the actual markup (they're just divs), but we'll likely change the styles in the future.

kiley-mitti commented 2 years ago

Cool! Not a problem Mark. Honestly we added it because I was brilliant enough to start testing in Checkbox and too stubborn to move on once I realized it didn't have every case we needed to test. I'll remove error-text and slot from checkbox but otherwise replicate on the rest!