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

Add a 'spinner' state to button #738

Closed
aquirkles opened this issue Sep 20, 2018 · 13 comments
Closed

Add a 'spinner' state to button #738

aquirkles opened this issue Sep 20, 2018 · 13 comments
Labels
owner: core team item created by tds member priority: medium Medium priority items to be taken on by the core team or contributors, next in queue. status: completed ✅ An item that has been closed, due to its completion type: enhancement enhancement or new requests

Comments

@aquirkles
Copy link
Contributor

aquirkles commented Sep 20, 2018

Description

The login page treatment to convey a 'processing' state is to overlay a spinner on the submit button while lightening the color, see screenshot:

screenshot-1

The dope jackalope team now has a need for this same component however it is not yet part of TDS, dope jack devs are willing to do this work and contribute to tds-core

Recommendation

Add an isLoading boolean prop that defaults to false which modifies the background color of the button appropriately and adds a spinner overlay:

variant="primary" -> background-color: $color-panache
variant="secondary" -> background-color: $color-white-lilac
variant="inverted" -> background-color: $color-athens-grey

Meta

  • TDS version: vX.Y.Z
  • Willing to develop solution: Yes
  • Has workaround: No
  • High impact: No
@githubjosh
Copy link
Contributor

I believe Team Bits has implemented this some time ago. In registration flow? May be able to leverage that.

@ah-arch @SandraFerns @aneesh-datta

@jraff
Copy link
Contributor

jraff commented Sep 20, 2018

Thanks for volunteering to contribute this feature. While an isLoading prop on <Button /> will work, we do not want to tightly couple <Button /> and <Spinner />. We currently have documentation explaining how you can overlay content with a <Spinner />. Something like this should work:

<Spinner spinner>
    <Button>Login</Button>
</Spinner>

However, <Spinner /> currently spans the width of its parent. We could add an option to <Spinner /> to display: inline-block.

@ah-arch
Copy link
Contributor

ah-arch commented Sep 20, 2018

Team Bits did just that, wrapping <Button> with a <Spinner> as mentioned above, e.g. for business-requests, but with additional css to center the spinner correctly.

Agree that it would be useful to standardized that in Button.

@aquirkles
Copy link
Contributor Author

@jraff so I'd be writing css to center the spinner and lighten the button background, this kind of runs against the philosophy of having a design system, no?

@Jeffrey-Chang
Copy link
Contributor

Jeffrey-Chang commented Sep 20, 2018

It seems like a lot more teams are now using this style of spinning on buttons.

Currently, 2 other teams are already using this with their own CSS written. Now a third team is using it and potentially even more teams in the future. It seems kind of wrong for each team to copy and paste the CSS from other teams and put it into their code base when we have a centralized design system.

This would also allow for inconsistencies across the teams/pages that are doing their own one off spinner on a button.

@jraff
Copy link
Contributor

jraff commented Sep 20, 2018

@aquirkles Here is a code recipe that can be used with the current version of TDS to achieve a <Spinner> on a <Button>.

<div style={{ display: 'inline-block' }}>
  <Spinner spinning>
    <Button>Login</Button>
  </Spinner>
</div>

Can you reach out to your design lead to see if this pattern is something that is consumable by every tribe at TELUS? If it is, it can be considered as a feature to be included in TDS core.

Another concern we have is related to accessibility, when the <Spinner> is present, the <Button> cannot be interacted with via click but can be via keyboard. Can you raise this with your design lead as well to determine what the interaction should be?

@aquirkles
Copy link
Contributor Author

So, this does in fact, produce the desired effect, which is great! But im not sure how id have found this out, maybe there should be a kind of TDS 'cookbook' or maybe theres documentation I should have look longer for? Either way, thanks.

@jraff
Copy link
Contributor

jraff commented Sep 20, 2018

No problem! A TDS cookbook is something we are currently looking into. telus/tds-community#87

@marcod1419 marcod1419 added type: enhancement enhancement or new requests owner: core team item created by tds member priority: medium Medium priority items to be taken on by the core team or contributors, next in queue. status: reviewed item that has been reviewed by the core team and is ready for action and removed triage labels Sep 26, 2018
@marcod1419
Copy link
Contributor

Update: We will be looking into creating a spinner that works inline so it can properly non-full-width buttons. Additionally, we will be looking into creating an accessible disabled button.

@lucylist
Copy link
Contributor

spoke to @Berjesty on this. the button has been implemented. she will confirm and get back to me.

@lucylist
Copy link
Contributor

hi @Jeffrey-Chang @Berjesty created these two separate tickets for the core team to make an official button loader.

https://telusdigital.atlassian.net/browse/TDS-1027 (making the button loader)
https://telusdigital.atlassian.net/browse/TDS-1027 (making spinner various sizes)

@jraff jraff mentioned this issue Dec 18, 2018
2 tasks
@theetrain
Copy link
Contributor

There is an enhancement in @tds/core-spinner@2.1.0 that provides an interface for wrapping inline elements:

image

<Spinner spinning inline>
  <Button>Login</Button>
</Spinner>

This issue will remain open until related tasks mentioned above are complete:

  1. multiple spinner sizes
  2. a disabled button interface

@theetrain
Copy link
Contributor

This conversation has been resolved. For a related topic on Accessibility with Spinner, see #907.

@theetrain theetrain added status: completed ✅ An item that has been closed, due to its completion and removed status: reviewed item that has been reviewed by the core team and is ready for action labels Apr 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
owner: core team item created by tds member priority: medium Medium priority items to be taken on by the core team or contributors, next in queue. status: completed ✅ An item that has been closed, due to its completion type: enhancement enhancement or new requests
Projects
None yet
Development

No branches or pull requests

8 participants