Text input loading state by mperrotti · Pull Request #1920 · primer/react

mperrotti · 2022-03-02T19:29:28Z

Issue: https://github.com/github/primer/issues/156

Adds a loading state to the text input. The loading indicator is positioned to avoid layout shifts in the component when it hides and shows.

`loadingIndicatorPosition="auto"` (default behavior)

Does not have a leadingVisual OR trailingVisual?
Show at the end of the input

Has a leadingVisual?
Show in place of the leadingVisual

Has a trailingVisual?
Show in place of the trailingVisual

Has a leadingVisualandtrailingVisual?
Show in place of the trailingVisual

Does not have a leadingVisual?
Show at the end of the input

`loadingIndicatorPosition="leading"`

Show in place of leadingVisual if there is one, or show at the start of the input

`loadingIndicatorPosition="trailing"`

Show in place of trailingVisual if there is one, or show at the end of the input

Screenshots

Merge checklist

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

changeset-bot · 2022-03-02T19:29:30Z

🦋 Changeset detected

Latest commit: f9ad318

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

This PR includes changesets to release 1 package

Name	Type
@primer/react	Minor

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

github-actions · 2022-03-02T19:32:44Z

size-limit report 📦

Path	Size
dist/browser.esm.js	63.6 KB (+0.64% 🔺)
dist/browser.umd.js	63.94 KB (+0.61% 🔺)

mperrotti · 2022-03-02T19:34:06Z

+ * 'leading': at the beginning of the input
+ * 'trailing': at the end of the input
+ **/
+ loadingIndicatorPosition?: 'auto' | 'leading' | 'trailing' // TODO: come up with a shorter name


Any better recommendations for this prop name? It's so long...

Maybe just loaderPosition?

siddharthkp · 2022-03-08T10:40:42Z

Link to API exercise

This is the option that we all liked:

<TextInputleadingVisual={MergeIcon}trailingVisual={CheckIcon}isLoading={true|false}loaderPosition="auto | leading | trailing"/>

…ding-state

Co-authored-by: Pavithra Kodmad <pksjce@github.com>

… into mp/text-input-loading-state

pksjce · 2022-03-10T08:26:18Z

@@ -0,0 +1,45 @@
+import React from 'react'
+import {Box, Spinner} from '.'
+import {TextInputNonPassthroughProps} from './TextInput'


do you think this is like a circular dependency?

Possibly, but it's just a type. Do you have any alternative suggestions?

pksjce · 2022-03-10T08:47:04Z

+import {Box, Spinner} from '.'
+import {TextInputNonPassthroughProps} from './TextInput'
+
+const TextInputInnerVisualSlot: React.FC<{


is it more like TextInputReplaceLoader or something like that.

I still feel like it's more about the visual than the loader.

siddharthkp

This is looking good! Left a few suggestions for the docs

What is the accessibility story for loading state? Does it need to be announced? Do we need to change the aria-describedby in FormControl?

…ding-state

mperrotti · 2022-03-10T16:49:05Z

What is the accessibility story for loading state? Does it need to be announced? Do we need to change the aria-describedby in FormControl?

I don't think we need to announce it, but I could be wrong. @alliethu - any input on this?

alliethu · 2022-03-10T17:10:37Z

What is the accessibility story for loading state? Does it need to be announced? Do we need to change the aria-describedby in FormControl?
I don't think we need to announce it, but I could be wrong. @alliethu - any input on this?

@mperrotti I believe you would still want to indicate a busy/loading status to SR users.

cc/ @primer/accessibility-reviewers

…ding-state

Co-authored-by: Cole Bemis <colebemis@github.com>

colebemis

Looks good! Left a few comments about docs but nothing blocking

…ding-state

mperrotti added 6 commits March 2, 2022 10:49

adds loading indicator to text inputs
c9f34f1

removes style debugging div
26976c7

updates stories and docs
5157087

updates tests
258ed6c

fixes silly test mistakes
245b568

revert default textinput story
6c1f358

mperrotti requested review from a team and siddharthkp March 2, 2022 19:29

adds changeset
81a1022

mperrotti commented Mar 2, 2022
View reviewed changes

Merge branch 'main' into mp/text-input-loading-state
89b0e44

mperrotti added the react label Mar 7, 2022

pksjce reviewed Mar 8, 2022
View reviewed changes

Comment threadsrc/TextInput.tsx Outdated

mperrottiand others added 4 commits March 8, 2022 11:36

Merge branch 'main' of github.com:primer/react into mp/text-input-loa…
e964c11
…ding-state

Update src/TextInput.tsx
5388500
Co-authored-by: Pavithra Kodmad <pksjce@github.com>

Merge branch 'mp/text-input-loading-state' of github.com:primer/react…
0473333
… into mp/text-input-loading-state

updates component API
070612f

pksjce reviewed Mar 10, 2022
View reviewed changes

siddharthkp reviewed Mar 10, 2022
View reviewed changes

Comment threaddocs/content/TextInput.mdx
Comment threaddocs/content/TextInput.mdx Outdated

mperrotti mentioned this pull request Mar 10, 2022
Add trailingAction to TextInput #1947
Merged
6 tasks

Merge branch 'main' of github.com:primer/react into mp/text-input-loa…
f2f52c7
…ding-state

addresses PR feedback
79fbe76

mperrotti added 2 commits March 10, 2022 12:12

fixes linting error
4a4fcc0

indicate a busy status to assistive technology
1bdf6d1

mperrotti requested review from pksjce and siddharthkp March 11, 2022 15:21

mperrotti added 2 commits March 11, 2022 10:40

Merge branch 'main' of github.com:primer/react into mp/text-input-loa…
5f14db8
…ding-state

updates snaps
3464982

colebemis reviewed Mar 14, 2022
View reviewed changes

Comment threaddocs/content/TextInput.mdx Outdated

mperrotti added 2 commits March 15, 2022 13:12

renames 'isLoading' prop to 'loading'
d586a80

Merge branch 'main' of github.com:primer/react into mp/text-input-loa…
ced3de3
…ding-state

mperrotti requested a review from colebemis March 15, 2022 17:13

siddharthkp assigned mperrotti Mar 16, 2022

colebemis reviewed Mar 21, 2022
View reviewed changes

Comment threaddocs/content/TextInput.mdx Outdated

colebemis reviewed Mar 21, 2022
View reviewed changes

Comment threaddocs/content/TextInput.mdx Outdated

colebemis reviewed Mar 21, 2022
View reviewed changes

Comment threaddocs/content/TextInput.mdx Outdated

colebemis reviewed Mar 21, 2022
View reviewed changes

Comment threaddocs/content/TextInput.mdx Outdated

colebemis reviewed Mar 21, 2022
View reviewed changes

Comment threaddocs/content/TextInput.mdx Outdated

mperrottiand others added 5 commits March 21, 2022 14:21

Update docs/content/TextInput.mdx
8bc639e
Co-authored-by: Cole Bemis <colebemis@github.com>

Update docs/content/TextInput.mdx
eebc472
Co-authored-by: Cole Bemis <colebemis@github.com>

Update docs/content/TextInput.mdx
2b18aee
Co-authored-by: Cole Bemis <colebemis@github.com>

Update docs/content/TextInput.mdx
52d2e1a
Co-authored-by: Cole Bemis <colebemis@github.com>

Update docs/content/TextInput.mdx
5cfc485
Co-authored-by: Cole Bemis <colebemis@github.com>

colebemis reviewed Mar 21, 2022
View reviewed changes

Comment threaddocs/content/TextInput.mdx Outdated

colebemis approved these changes Mar 21, 2022
View reviewed changes

mperrotti added 6 commits March 21, 2022 14:31

Merge branch 'main' of github.com:primer/react into mp/text-input-loa…
fc0eeef
…ding-state

updates snapshots
8bf281d

nests examples under an 'Examples' heading
e785ccd

export TextInput non-pass-through props
a3dc21d

fix undefined type usage
fffa764

fixes story types
f9ad318

mperrotti merged commit 40ed423 into mainMar 21, 2022

mperrotti deleted the mp/text-input-loading-state branch March 21, 2022 19:33

primer-css mentioned this pull request Mar 21, 2022
Release tracking #1966
Merged

Uh oh!

Conversation

mperrotti commented Mar 2, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

loadingIndicatorPosition="auto" (default behavior)

loadingIndicatorPosition="leading"

loadingIndicatorPosition="trailing"

Screenshots

Merge checklist

Uh oh!

changeset-botBot commented Mar 2, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

github-actionsBot commented Mar 2, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

size-limit report 📦

Uh oh!

mperrottiMar 2, 2022

Choose a reason for hiding this comment

Uh oh!

pksjceMar 8, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

siddharthkp commented Mar 8, 2022

Uh oh!

pksjceMar 10, 2022

Choose a reason for hiding this comment

Uh oh!

mperrottiMar 10, 2022

Choose a reason for hiding this comment

Uh oh!

pksjceMar 10, 2022

Choose a reason for hiding this comment

Uh oh!

mperrottiMar 10, 2022

Choose a reason for hiding this comment

Uh oh!

siddharthkp left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mperrotti commented Mar 10, 2022

Uh oh!

alliethu commented Mar 10, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

colebemis left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mperrotti commented Mar 2, 2022•
edited
Loading

`loadingIndicatorPosition="auto"` (default behavior)

`loadingIndicatorPosition="leading"`

`loadingIndicatorPosition="trailing"`

changeset-botBot commented Mar 2, 2022•
edited
Loading

github-actionsBot commented Mar 2, 2022•
edited
Loading

siddharthkp left a comment •
edited
Loading