Enhancement/9385 persist current selection #9661
Conversation
|
Build files for 653c847 are ready:
|
|
Size Change: +8.63 kB (+0.46%) Total Size: 1.89 MB
ℹ️ View Unchanged
|
| @@ -70,9 +70,7 @@ export default function Footer( { | |||
| KEY_METRICS_SELECTED | |||
| ) | |||
| ); | |||
| const keyMetricsSettings = useSelect( ( select ) => | |||
There was a problem hiding this comment.
The old error text shown inline, is replaced with new better looking UI error component. It is not guarded behind feature flag since it is a change to the selection panel footer, which is global component, not having logic related to the ACR, and new error component looks better
| @@ -39,18 +39,14 @@ import { __, sprintf } from '@wordpress/i18n'; | |||
| */ | |||
| import { useSelect } from 'googlesitekit-data'; | |||
| import { Button, SpinnerButton } from 'googlesitekit-components'; | |||
| import ErrorNotice from '../ErrorNotice'; | |||
There was a problem hiding this comment.
Also here as above, removed inline error text related data
| @@ -0,0 +1,191 @@ | |||
| /** | |||
| * Selection Panel Footer component. | |||
| * | |||
There was a problem hiding this comment.
Included key metrics specific component, in order to override/remove the inline error, since global one is used in Audience segmentation as well. The new error component added in this PR can be reused there as well for consistency, but is outside of scope of this PR. Will ping team M about this
| @@ -22,10 +22,6 @@ | |||
|
|
|||
| .googlesitekit-selection-box { | |||
|
|
|||
| .mdc-checkbox__description { | |||
| padding-top: 4px; | |||
There was a problem hiding this comment.
Only ting removed from audience segmentation style to match the spacing between content for consistency
| min-height: 28px; | ||
| padding: 6px 14px; | ||
|
|
||
| --mdc-ripple-fg-opacity: 0; |
There was a problem hiding this comment.
This is to prevent the flickering
| /* translators: %1$s: Error message from Google API. */ | ||
| __( '%1$s%2$s Please try again.', 'google-site-kit' ), | ||
| message, | ||
| message.endsWith( '.' ) ? '' : '.' |
There was a problem hiding this comment.
There was a bug with this component, where additional period was included even to the messages with period, so I fixed it here, as it is used for key metrics saving errors, and 2 dots (..) were appearing
| ) | ||
| ) } | ||
| { isMobileBreakpoint && ( | ||
| <TabBar |
There was a problem hiding this comment.
To support proper horizontal scroll on mobile devices for tabs, existing TabBar component is used for simplicity, since it is already providing everything needed to make the scroll smooth and cross browser compatible, RTL supported, etc. No need to reinvent the wheel
|
@techanvil Tagging you for visibility, this is the PR I mentioned on standup, there is small spacing change and alignment on global component that affect Audience segmentation panel only (you can check it in VRT images), we have new error style, I included it globally in case you want to adapt it for consistency you can see the usage and component in this PR |
Summary
Addresses issue:
Relevant technical choices
PR Author Checklist
Do not alter or remove anything below. The following sections will be managed by moderators only.
Code Reviewer Checklist
Merge Reviewer Checklist