Skip to content

Fix spinner button animation for long action text#8472

Merged
aduth merged 7 commits intomainfrom
aduth-fix-spinner-button
May 24, 2023
Merged

Fix spinner button animation for long action text#8472
aduth merged 7 commits intomainfrom
aduth-fix-spinner-button

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented May 23, 2023

🛠 Summary of changes

Fixes an issue where SpinnerButton may appear incorrect when using longer action_message text. The issue is that the spinner button's container width will grow to accommodate the longer text, but this growth causes the spinner animation to be positioned incorrectly, since it is horizontally centered.

📜 Testing Plan

  1. Go to http://localhost:3000/components/inspect/spinner_button/workbench?action_message=This+is+a+long+action+message.
  2. Click button
  3. Observe spinner animation is centered in the button, and the action message appears after 15 seconds

👀 Screenshots

Before After
image image

Note the issue is horizontal centering:

image

changelog: Bug Fixes, UI Components, Fix appearance of button spinner in specific scenarios
@aduth aduth requested a review from nickttng May 23, 2023 19:29

get button(): HTMLElement {
return this.querySelector('a,button:not([type]),[type="submit"],[type="button"]')!;
return this.querySelector('.usa-button')!;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suspect this was premature optimization on past Andrew's part. I don't know that we ever render any spinner buttons that aren't .usa-button.

aduth added 3 commits May 24, 2023 10:30
Since we explicitly handle outline button as part of the spinner button implementation
@aduth
Copy link
Contributor Author

aduth commented May 24, 2023

In testing this, I forgot there's a React implementation of this as well, so I'll need to sync the markup changes there as well.

aduth and others added 2 commits May 24, 2023 13:53
Revert stylesheet to main application stylesheet, since currently there's not a way to auto-load component stylesheets for components implemented in React
See: https://github.com/18F/identity-idp/pull/8472/files#r1204466631

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
@aduth
Copy link
Contributor Author

aduth commented May 24, 2023

In testing this, I forgot there's a React implementation of this as well, so I'll need to sync the markup changes there as well.

Updated in b51d60a. I had to move styles back to the shared stylesheet, since we currently can't use #8375's per-component stylesheets for components which also have a React implementation.

@aduth aduth merged commit e662471 into main May 24, 2023
@aduth aduth deleted the aduth-fix-spinner-button branch May 24, 2023 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants