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

User Input - New Design - More Updates #6238

Merged
merged 3 commits into from
Dec 2, 2022

Conversation

nfmohit
Copy link
Collaborator

@nfmohit nfmohit commented Dec 1, 2022

Summary

Addresses issue:

Relevant technical choices

This PR addresses this comment and does the following changes:

  • Updates the Progress Bar logic so that it is at 75% in the final question, and 100% at the preview screen.
  • Removes the initial focus on the first option when a question is unanswered.

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
Copy link

github-actions bot commented Dec 1, 2022

Size Change: +108 B (0%)

Total Size: 1.5 MB

Filename Size Change
./dist/assets/js/googlesitekit-settings-********************.js 49.1 kB +106 B (0%)
./dist/assets/js/googlesitekit-user-input-********************.js 42.9 kB +2 B (0%)
ℹ️ View Unchanged
Filename Size
./dist/assets/css/googlesitekit-admin-css-********************.min.css 49 kB
./dist/assets/css/googlesitekit-adminbar-css-********************.min.css 11.1 kB
./dist/assets/css/googlesitekit-wp-dashboard-css-********************.min.css 6.17 kB
./dist/assets/js/33-********************.js 2.8 kB
./dist/assets/js/34-********************.js 2.28 kB
./dist/assets/js/35-********************.js 3.72 kB
./dist/assets/js/36-********************.js 51.9 kB
./dist/assets/js/37-********************.js 16.1 kB
./dist/assets/js/38-********************.js 70.9 kB
./dist/assets/js/39-********************.js 31.6 kB
./dist/assets/js/40-********************.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 35.1 kB
./dist/assets/js/googlesitekit-api-********************.js 9.29 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.09 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.2 kB
./dist/assets/js/googlesitekit-datastore-user-********************.js 22.9 kB
./dist/assets/js/googlesitekit-entity-dashboard-********************.js 63.3 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 73.8 kB
./dist/assets/js/googlesitekit-modules-********************.js 20.1 kB
./dist/assets/js/googlesitekit-modules-adsense-********************.js 68.7 kB
./dist/assets/js/googlesitekit-modules-analytics-4-********************.js 19.4 kB
./dist/assets/js/googlesitekit-modules-analytics-********************.js 70.4 kB
./dist/assets/js/googlesitekit-modules-idea-hub-********************.js 26.8 kB
./dist/assets/js/googlesitekit-modules-optimize-********************.js 18.7 kB
./dist/assets/js/googlesitekit-modules-pagespeed-insights-********************.js 19.2 kB
./dist/assets/js/googlesitekit-modules-search-console-********************.js 37.8 kB
./dist/assets/js/googlesitekit-modules-tagmanager-********************.js 31.8 kB
./dist/assets/js/googlesitekit-modules-thank-with-google-********************.js 24.9 kB
./dist/assets/js/googlesitekit-polyfills-********************.js 379 B
./dist/assets/js/googlesitekit-splash-********************.js 68.5 kB
./dist/assets/js/googlesitekit-vendor-********************.js 330 kB
./dist/assets/js/googlesitekit-widgets-********************.js 14.9 kB
./dist/assets/js/googlesitekit-wp-dashboard-********************.js 57.7 kB
./dist/assets/js/runtime-********************.js 1.34 kB

compressed-size-action

@aaemnnosttv
Copy link
Collaborator

@nfmohit when creating follow-up PRs for the release, please remember to update the target branch as it always defaults to develop 👍

@aaemnnosttv aaemnnosttv changed the base branch from develop to main December 2, 2022 04:42
Copy link
Collaborator

@aaemnnosttv aaemnnosttv left a comment

Choose a reason for hiding this comment

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

Looks like this PR was created from develop which includes many more changes than it should. Would you please recreate this from main?

@nfmohit nfmohit force-pushed the enhancement/#5890-design-followup branch from 8d1f96c to f2ea846 Compare December 2, 2022 07:32
@nfmohit
Copy link
Collaborator Author

nfmohit commented Dec 2, 2022

@nfmohit when creating follow-up PRs for the release, please remember to update the target branch as it always defaults to develop 👍

@aaemnnosttv My apologies, I missed it this time. I'll keep it in mind, thank you for correcting it and the heads-up!

Looks like this PR was created from develop which includes many more changes than it should. Would you please recreate this from main?

I have rebased this branch and removed the unnecessary changes, leaving only the necessary change. Does it look good now?

Thanks!

Copy link
Collaborator

@aaemnnosttv aaemnnosttv left a comment

Choose a reason for hiding this comment

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

Thanks for rebasing @nfmohit that's perfectly fine with me.

Let's keep this PR focused on the change to the progress bar and we can revisit a11y as a primary focus in a new issue.

Comment on lines 80 to 84
} else {
const el = optionsRef.current.querySelector(
`input[type="${ optionType }"]`
);
focusOption( el );
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's not change this here. It's out of scope for the issue and I'm not sure if we should. Accessibility should probably be reviewed separately once the design iterations are implemented (or nearly done).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Understood, reverted this change. Thank you for confirming, @aaemnnosttv!

Copy link
Collaborator

@aaemnnosttv aaemnnosttv left a comment

Choose a reason for hiding this comment

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

Great, thanks @nfmohit!

@aaemnnosttv aaemnnosttv merged commit 2a24f57 into main Dec 2, 2022
@aaemnnosttv aaemnnosttv deleted the enhancement/#5890-design-followup branch December 2, 2022 15:50
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.

2 participants