[User Input] Admin settings design #6332

nfmohit · 2022-12-15T08:10:30Z

Summary

Addresses issue:

Implement new design for User Input Settings #5896

Relevant technical choices

This PR updates the User Input admin settings design to match Figma.

PR Author Checklist

My code is tested and passes existing unit tests.
My code has an appropriate set of unit tests which all pass.
My code is backward-compatible with WordPress 5.2 and PHP 5.6.
My code follows the WordPress coding standards.
My code has proper inline documentation.
I have added a QA Brief on the issue linked above.
I have signed the Contributor License Agreement (see https://cla.developers.google.com/).

Do not alter or remove anything below. The following sections will be managed by moderators only.

Code Reviewer Checklist

Run the code.
Ensure the acceptance criteria are satisfied.
Reassess the implementation with the IB.
Ensure no unrelated changes are included.
Ensure CI checks pass.
Check Storybook where applicable.
Ensure there is a QA Brief.

Merge Reviewer Checklist

Ensure the PR has the correct target branch.
Double-check that the PR is okay to be merged.
Ensure the corresponding issue has a ZenHub release assigned.
Add a changelog message to the issue.

github-actions · 2022-12-15T08:20:35Z

Size Change: +6 B (0%)

Total Size: 1.36 MB

Filename	Size	Change
`./dist/assets/css/googlesitekit-admin-css-********************.min.css`	48.6 kB	+10 B (0%)
`./dist/assets/js/googlesitekit-settings-********************.js`	50.7 kB	-23 B (0%)
`./dist/assets/js/googlesitekit-user-input-********************.js`	42.8 kB	+19 B (0%)

ℹ️ View Unchanged

Filename	Size
`./dist/assets/css/googlesitekit-adminbar-css-********************.min.css`	11 kB
`./dist/assets/css/googlesitekit-wp-dashboard-css-********************.min.css`	7.24 kB
`./dist/assets/js/32-********************.js`	2.8 kB
`./dist/assets/js/33-********************.js`	2.28 kB
`./dist/assets/js/34-********************.js`	3.72 kB
`./dist/assets/js/35-********************.js`	51.9 kB
`./dist/assets/js/36-********************.js`	3.12 kB
`./dist/assets/js/analytics-advanced-tracking-********************.js`	769 B
`./dist/assets/js/googlesitekit-activation-********************.js`	27 kB
`./dist/assets/js/googlesitekit-adminbar-********************.js`	34.9 kB
`./dist/assets/js/googlesitekit-api-********************.js`	9.34 kB
`./dist/assets/js/googlesitekit-base-********************.js`	1.13 kB
`./dist/assets/js/googlesitekit-components-gm2-********************.js`	5.82 kB
`./dist/assets/js/googlesitekit-components-gm3-********************.js`	421 B
`./dist/assets/js/googlesitekit-data-********************.js`	2.12 kB
`./dist/assets/js/googlesitekit-datastore-forms-********************.js`	9.15 kB
`./dist/assets/js/googlesitekit-datastore-location-********************.js`	2.08 kB
`./dist/assets/js/googlesitekit-datastore-site-********************.js`	15.3 kB
`./dist/assets/js/googlesitekit-datastore-ui-********************.js`	9.26 kB
`./dist/assets/js/googlesitekit-datastore-user-********************.js`	22.9 kB
`./dist/assets/js/googlesitekit-entity-dashboard-********************.js`	63.6 kB
`./dist/assets/js/googlesitekit-i18n-********************.js`	3.92 kB
`./dist/assets/js/googlesitekit-idea-hub-notice-********************.js`	45 kB
`./dist/assets/js/googlesitekit-idea-hub-post-list-********************.js`	24.8 kB
`./dist/assets/js/googlesitekit-main-dashboard-********************.js`	71.8 kB
`./dist/assets/js/googlesitekit-modules-adsense-********************.js`	69.5 kB
`./dist/assets/js/googlesitekit-modules-analytics-********************.js`	71.1 kB
`./dist/assets/js/googlesitekit-modules-analytics-4-********************.js`	19.4 kB
`./dist/assets/js/googlesitekit-modules-********************.js`	20.6 kB
`./dist/assets/js/googlesitekit-modules-idea-hub-********************.js`	27.1 kB
`./dist/assets/js/googlesitekit-modules-optimize-********************.js`	18.7 kB
`./dist/assets/js/googlesitekit-modules-pagespeed-insights-********************.js`	20 kB
`./dist/assets/js/googlesitekit-modules-search-console-********************.js`	38.3 kB
`./dist/assets/js/googlesitekit-modules-tagmanager-********************.js`	31.9 kB
`./dist/assets/js/googlesitekit-polyfills-********************.js`	379 B
`./dist/assets/js/googlesitekit-splash-********************.js`	68.7 kB
`./dist/assets/js/googlesitekit-vendor-********************.js`	329 kB
`./dist/assets/js/googlesitekit-widgets-********************.js`	15.4 kB
`./dist/assets/js/googlesitekit-wp-dashboard-********************.js`	59.7 kB
`./dist/assets/js/runtime-********************.js`	1.28 kB

_{compressed-size-action}

nfmohit · 2022-12-15T08:39:03Z

Note for Code Reviewer: The VRT check failure is most likely related to #6324. I’ve checked the report and there are no relevant scenarios failing.

hussain-t

Nice one, @nfmohit. LGTM 👍

tofumatt

Following the QA Brief, the CTA doesn't seem to be appearing properly here… it shows for a second, but then disappears:

CleanShot.2022-12-21.at.21.56.18.mp4

I also noticed that an Analytics request is being made when Analytics isn't enabled:

I think that might be related to the CTA not appearing at all (not even for a split-second) when Analytics isn't connected:

CleanShot.2022-12-21.at.21.58.38.mp4

Everything else seems fine and the changes in the PR don't even seem directly related to those issues, so if that means we should re-open an old issue or create a new one let me know 🙂

nfmohit · 2022-12-22T06:21:24Z

Hi, @tofumatt! Thank you for the kind review here.

the CTA doesn't seem to be appearing properly here… it shows for a second, but then disappears

It looks like @wpdarren noticed this here in #5895 but no one was able to replicate it further later on. I have added a comment there.

I also noticed that an Analytics request is being made when Analytics isn't enabled

This was also reported here in #5895 and @derweili suggests this should be addressed in #5933.

How do you think we should proceed here? Thanks!

nfmohit · 2022-12-22T11:29:53Z

@tofumatt Update: @wpdarren will be creating an issue about the CTA disappearing as mentioned here.

tofumatt

I see the Key Metrics not appearing is filed as #6370, but the current QA Brief mentions looking for that element.

I'm thinking the QA Brief here should be adjusted, but should we consider #6370 as blocking this issue or not? Let me know what you think.

If it's fine to QA the Key Metrics Admin Settings link separately I'm cool with that 🙂

nfmohit · 2023-01-06T09:56:39Z

I see the Key Metrics not appearing is filed as #6370, but the current QA Brief mentions looking for that element.

I'm thinking the QA Brief here should be adjusted, but should we consider #6370 as blocking this issue or not? Let me know what you think.

If it's fine to QA the Key Metrics Admin Settings link separately I'm cool with that 🙂

That's indeed a very valid concern, @tofumatt. I believe it'd be ideal to wait until #6370 is merged, however, keeping in mind that this issue also blocks a large #5897, I have updated the QAB to bypass the check for the CTA.

Let me know what you think. Thanks!

tofumatt

The current QA brief mentions seeing this CTA in the Admin Settings, but I never see it:

CleanShot.2023-01-06.at.14.22.41.mp4

I see you added an addendum about it often not appearing, but we shouldn't have "maybes" in QA Briefs 😆. Also, it's quite far down in the brief so is a bit confusing.

Let's remove it from there for now before sending this to QA. The QA brief should only have exactly what can be verifiably tested and reported on.

nfmohit · 2023-01-06T14:30:27Z

Understood. Removed this part from the QAB. Thank you @tofumatt!

tofumatt

Awesome, thanks! In that case, looks good 👍🏻

nfmohit added 5 commits December 15, 2022 13:59

Update settings section title.

a6303df

Update settings section sub-heading.

4eb24fa

Update UserInputPreview to conditionally hide header.

3b4d687

Do not display UserInputPreview header in Admin Settings.

9f178b9

Update user input settings styles to match Figma.

073a786

hussain-t approved these changes Dec 15, 2022

View reviewed changes

tofumatt requested changes Dec 21, 2022

View reviewed changes

nfmohit mentioned this pull request Dec 22, 2022

Add CTA for users with Incomplete User Input answers in Settings page #5895

Closed

nfmohit requested a review from tofumatt December 22, 2022 06:21

tofumatt reviewed Jan 5, 2023

View reviewed changes

nfmohit requested a review from tofumatt January 6, 2023 09:56

tofumatt requested changes Jan 6, 2023

View reviewed changes

nfmohit requested a review from tofumatt January 6, 2023 14:43

tofumatt approved these changes Jan 10, 2023

View reviewed changes

tofumatt merged commit 9aebfed into develop Jan 10, 2023

tofumatt deleted the enhancement/#5896-user-input-settings-design branch January 10, 2023 21:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[User Input] Admin settings design #6332

[User Input] Admin settings design #6332

nfmohit commented Dec 15, 2022 •

edited by hussain-t

Loading

github-actions bot commented Dec 15, 2022

nfmohit commented Dec 15, 2022

hussain-t left a comment

tofumatt left a comment •

edited

Loading

nfmohit commented Dec 22, 2022

nfmohit commented Dec 22, 2022

tofumatt left a comment

nfmohit commented Jan 6, 2023

tofumatt left a comment

nfmohit commented Jan 6, 2023

tofumatt left a comment

[User Input] Admin settings design #6332

[User Input] Admin settings design #6332

Conversation

nfmohit commented Dec 15, 2022 • edited by hussain-t Loading

Summary

Relevant technical choices

PR Author Checklist

Code Reviewer Checklist

Merge Reviewer Checklist

github-actions bot commented Dec 15, 2022

nfmohit commented Dec 15, 2022

hussain-t left a comment

Choose a reason for hiding this comment

tofumatt left a comment • edited Loading

Choose a reason for hiding this comment

nfmohit commented Dec 22, 2022

nfmohit commented Dec 22, 2022

tofumatt left a comment

Choose a reason for hiding this comment

nfmohit commented Jan 6, 2023

tofumatt left a comment

Choose a reason for hiding this comment

nfmohit commented Jan 6, 2023

tofumatt left a comment

Choose a reason for hiding this comment

nfmohit commented Dec 15, 2022 •

edited by hussain-t

Loading

tofumatt left a comment •

edited

Loading