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

website: Update input documentation & demos #1180

Merged
merged 15 commits into from
Apr 7, 2023

Conversation

FlyersPh9
Copy link
Collaborator

  • Add input demos for the different options.
  • Update documentation, include adding hyperlinks to other mentioned components.

@FlyersPh9 FlyersPh9 added the documentation Improvements or additions to documentation label Mar 31, 2023
@FlyersPh9 FlyersPh9 requested a review from a team as a code owner March 31, 2023 19:50
@FlyersPh9 FlyersPh9 self-assigned this Mar 31, 2023
@FlyersPh9 FlyersPh9 requested review from a team, mayank99 and elephantcatdog and removed request for a team March 31, 2023 19:50
apps/website/src/pages/docs/input.mdx Outdated Show resolved Hide resolved
<AllExamples.InputInputExample client:load />
</LiveExample>

Use `<Input>` when all that is needed is a text field with no label, hint text, or status. In most scenarios we strongly recommend an input being paired with a label.
Copy link
Contributor

Choose a reason for hiding this comment

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

"In most scenarios" is not right. Really we want it to be all scenarios. It's an accessibility requirement.

The use case for <Input> (over <LabeledInput>) is when using a separate <Label> or a hidden label (either using <VisuallyHidden> or aria-label).

Copy link
Contributor

Choose a reason for hiding this comment

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

Continuing that logic, I'm not sure it makes sense to separate these two components as "variants". Instead, we could assume <LabeledInput> throughout the page, and then have a section (under Usage) for a "Standalone input".

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've removed the "In most scenarios" and explicitly stated that a label is an accessibility requirement. I've also removed variants and added a standalone section.

Copy link
Collaborator Author

@FlyersPh9 FlyersPh9 Apr 6, 2023

Choose a reason for hiding this comment

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

Most search inputs don't have a label, does search somehow get an exception?

Copy link
Contributor

Choose a reason for hiding this comment

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

If the input has other contextual hints to help the user enter the right text (e.g. a search icon in the case of a search input, then a visible label can be omitted, but there still needs to be a hidden label. This hidden label could be associated using aria-labelledlby or <label> + VisuallyHidden for example.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good to know, I'll try using <label> + VisuallyHidden on the search boxes in the demo pages of #1073.

apps/website/src/pages/docs/input.mdx Outdated Show resolved Hide resolved
apps/website/src/pages/docs/input.mdx Outdated Show resolved Hide resolved
apps/website/src/examples/Input.button.tsx Outdated Show resolved Hide resolved
apps/website/src/examples/Input.button.tsx Outdated Show resolved Hide resolved
apps/website/src/pages/docs/input.mdx Outdated Show resolved Hide resolved
apps/website/src/pages/docs/input.mdx Outdated Show resolved Hide resolved
apps/website/src/pages/docs/input.mdx Outdated Show resolved Hide resolved
@mayank99
Copy link
Contributor

mayank99 commented Apr 4, 2023

There are some merge conflicts.

@FlyersPh9 FlyersPh9 mentioned this pull request Apr 7, 2023
21 tasks
@FlyersPh9 FlyersPh9 enabled auto-merge April 7, 2023 19:27
@FlyersPh9 FlyersPh9 added this pull request to the merge queue Apr 7, 2023
Merged via the queue into main with commit 131fa2e Apr 7, 2023
@FlyersPh9 FlyersPh9 deleted the jon/documentation-site-input branch April 7, 2023 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants