Settings screen: improve discoverability of save button #5443
Conversation
|
Plugin builds for 118dba3 are ready 🛎️!
|
|
@johnwatkins0 As I chatted to you, there's a slight change in the height of the fixed bar when the notice is shown.
|
Fixed in 327d298 |
There was a problem hiding this comment.
Works great for me! I will defer to @pierlon for confirming the JS changes.
| @media screen and (min-width: 783px) { | ||
| padding: 30px 90px; | ||
| } |
There was a problem hiding this comment.
Minor point: should files like this eventually get renamed to *.sass? This isn't valid CSS so my IDE complains:
| { error && <ErrorNotice errorMessage={ error.message || __( 'An error occurred. You might be offline or logged out.', 'amp' ) } /> } | ||
| { ! error && ! hasOptionsChanges && didSaveOptions && ! downloadingTheme && ( | ||
| <AMPNotice | ||
| className={ `amp-save-success-notice ${ savedNoticeClass }` } |
There was a problem hiding this comment.
Shouldn't the notice be hidden once savedNoticeClass is set to dismissed?
There was a problem hiding this comment.
Removed the class entirely in 9d1ec07
| </svg> | ||
| </Button> | ||
| { error && <ErrorNotice errorMessage={ error.message || __( 'An error occurred. You might be offline or logged out.', 'amp' ) } /> } | ||
| { ! error && ! hasOptionsChanges && didSaveOptions && ! downloadingTheme && ( |
There was a problem hiding this comment.
Is it now the case that the notice is always to be shown unless this condition evaluates to false?
There was a problem hiding this comment.
Yeah, it shows until the user makes changes again. So I just pushed 9d1ec07 to take this a step further and not have a custom class on the notice at all. That was to handle the fly in and automatic timeout, which aren't needed anymore.
|
Also, I noticed an error in one of the CSS files while reviewing (not related to these changes). Here's a patch with the fix: --- assets/src/css/core-components.css (revision 5b4b10be6bbf0729ec2eff5fce372d050466b367)
+++ assets/src/css/core-components.css (date 1601691170646)
@@ -77,7 +77,7 @@
.amp .components-button.is-primary:disabled.is-busy {
background-size: 100% 100%;
- background-image: linear-gradient(-45deg, var(--amp-settings-color-brand) 28%, rgba(36, 89, 231, .8) 28%, rgb(36, 89, 231, .8) 72%, var(--amp-settings-color-brand) 72%);
+ background-image: linear-gradient(-45deg, var(--amp-settings-color-brand) 28%, rgba(36, 89, 231, .8) 28%, rgba(36, 89, 231, .8) 72%, var(--amp-settings-color-brand) 72%);
border-color: var(--color-gray-medium);
} |
Fixed in 4684790 |
|
|
Summary
Fixes #5353
On the settings screen, this moves the "Save" button and the related success and error notices into a white bar fixed to the bottom of the screen. I have reused styles from the similar component in the onboarding wizard.
Result with no error:
Result with error:
Checklist