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

Update/gutenboarding hide next button in intent gathering step #38705

Conversation

andrewserong
Copy link
Member

@andrewserong andrewserong commented Jan 8, 2020

Changes proposed in this Pull Request

  • Only show the Next button at the top right of the screen if there is a value for Next and a design has been selected.
  • Change the label for Next to "Create my site".
  • Switch the setSelectedDesign behaviour to use the onboard wordpress/data store instead of React state hook.
  • The "Create my site" button currently just sends the user back to the design step, since we don't (yet) have somewhere for this button to go

Before

image

After

image

Testing instructions

  • Go to http://calypso.localhost:3000/gutenboarding and in the top right hand corner there shouldn't be a Next button
  • Fill out the site topic and site name and click Continue
  • On the /gutenboarding/design step, after you select a design, you should see the Create my site button at the top right
  • Click Back and on the IntentGathering step (the first step), the Next button should not be present at the top right.

Fixes #38698

@andrewserong andrewserong requested a review from a team as a code owner January 8, 2020 01:21
@matticbot
Copy link
Contributor

@andrewserong andrewserong self-assigned this Jan 8, 2020
@andrewserong andrewserong added [Goal] Gutenberg Working towards full integration with Gutenberg [Feature Group] Signup & Site Onboarding Tools for user registration and onboarding new users to the site. [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Jan 8, 2020
@matticbot
Copy link
Contributor

matticbot commented Jan 8, 2020

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Webpack Runtime (~30923 bytes added 📈 [gzipped])

name      parsed_size         gzip_size
manifest    +169094 B  (new)   +30923 B  (new)

Webpack runtime for loading modules. It is included in the HTML page as an inline script. Is downloaded and parsed every time the app is loaded.

App Entrypoints (~1309870 bytes added 📈 [gzipped])

name                   parsed_size         gzip_size
entry-gutenboarding     +1824200 B  (new)  +489138 B  (new)
entry-main              +1607830 B  (new)  +391613 B  (new)
entry-login             +1091349 B  (new)  +286123 B  (new)
entry-domains-landing    +536045 B  (new)  +142996 B  (new)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~5685658 bytes added 📈 [gzipped])

name                      parsed_size         gzip_size
woocommerce                +2113543 B  (new)  +549773 B  (new)
post-editor                +1915694 B  (new)  +522764 B  (new)
checkout                   +1150938 B  (new)  +284618 B  (new)
purchases                   +953852 B  (new)  +224000 B  (new)
domains                     +841188 B  (new)  +190459 B  (new)
stats                       +785337 B  (new)  +194870 B  (new)
gutenberg-editor            +691426 B  (new)  +191209 B  (new)
jetpack-connect             +551007 B  (new)  +144051 B  (new)
comments                    +510285 B  (new)  +116999 B  (new)
settings                    +506583 B  (new)  +127868 B  (new)
plugins                     +477730 B  (new)  +118534 B  (new)
help                        +464305 B  (new)  +114645 B  (new)
activity                    +462549 B  (new)  +115233 B  (new)
settings-writing            +448095 B  (new)  +110482 B  (new)
plans                       +429808 B  (new)  +110596 B  (new)
marketing                   +424583 B  (new)  +103296 B  (new)
security                    +412653 B  (new)  +106490 B  (new)
theme                       +394617 B  (new)   +97891 B  (new)
reader                      +393015 B  (new)  +100858 B  (new)
media                       +366173 B  (new)   +96295 B  (new)
people                      +354625 B  (new)   +88405 B  (new)
themes                      +327273 B  (new)   +83870 B  (new)
notification-settings       +308041 B  (new)   +77186 B  (new)
account                     +302979 B  (new)   +79887 B  (new)
concierge                   +298453 B  (new)   +69980 B  (new)
checklist                   +288720 B  (new)   +75291 B  (new)
email                       +284769 B  (new)   +72662 B  (new)
account-close               +281425 B  (new)   +74273 B  (new)
posts                       +281121 B  (new)   +71326 B  (new)
posts-custom                +278790 B  (new)   +70649 B  (new)
google-my-business          +270431 B  (new)   +72658 B  (new)
zoninator                   +262730 B  (new)   +70579 B  (new)
earn                        +262638 B  (new)   +65190 B  (new)
home                        +253547 B  (new)   +65683 B  (new)
happychat                   +246628 B  (new)   +66265 B  (new)
site-blocks                 +243185 B  (new)   +64139 B  (new)
hosting                     +241006 B  (new)   +62875 B  (new)
pages                       +239149 B  (new)   +63437 B  (new)
wp-super-cache              +224336 B  (new)   +55757 B  (new)
privacy                     +223672 B  (new)   +59227 B  (new)
me                          +218539 B  (new)   +57803 B  (new)
settings-security           +216207 B  (new)   +54806 B  (new)
settings-performance        +206719 B  (new)   +54435 B  (new)
import                      +205701 B  (new)   +52901 B  (new)
accept-invite               +180721 B  (new)   +47295 B  (new)
export                      +171146 B  (new)   +45005 B  (new)
customize                   +170841 B  (new)   +46715 B  (new)
settings-discussion         +169534 B  (new)   +42677 B  (new)
signup                      +161232 B  (new)   +40055 B  (new)
devdocs                     +153161 B  (new)   +40627 B  (new)
feature-upsell              +144369 B  (new)   +34093 B  (new)
migrate                     +109500 B  (new)   +29092 B  (new)
hello-dolly                  +95484 B  (new)   +26034 B  (new)
preview                      +92468 B  (new)   +25224 B  (new)
sensei                       +90972 B  (new)   +24476 B  (new)
sites                        +86323 B  (new)   +23225 B  (new)
auth                         +18635 B  (new)    +5338 B  (new)
domain-connect-authorize     +12903 B  (new)    +3660 B  (new)
mailing-lists                 +6719 B  (new)    +1927 B  (new)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~2590240 bytes added 📈 [gzipped])

name                                                         parsed_size         gzip_size
async-load-design-blocks                                      +2727205 B  (new)  +631939 B  (new)
async-load-design                                             +1789951 B  (new)  +409980 B  (new)
async-load-design-playground                                  +1705445 B  (new)  +389230 B  (new)
async-load-components-web-preview-component                    +344460 B  (new)   +87022 B  (new)
async-load-my-sites-site-settings-seo-settings-form            +280466 B  (new)   +74713 B  (new)
async-load-post-editor-media-modal                             +274396 B  (new)   +72376 B  (new)
async-load-blocks-calendar-popover                             +240416 B  (new)   +46772 B  (new)
async-load-blocks-inline-help-popover                          +236463 B  (new)   +56389 B  (new)
async-load-signup-steps-domains                                +206620 B  (new)   +49278 B  (new)
async-load-signup-steps-plans                                  +155349 B  (new)   +40614 B  (new)
async-load-signup-steps-clone-point                            +152683 B  (new)   +35160 B  (new)
async-load-my-sites-checklist-wpcom-checklist-component-jsx    +132783 B  (new)   +32099 B  (new)
async-load-emoji-text                                          +128897 B  (new)   +22494 B  (new)
async-load-reader-following-manage                             +125397 B  (new)   +32054 B  (new)
async-load-apps-notifications-index-jsx                        +119629 B  (new)   +34787 B  (new)
async-load-signup-steps-user                                   +112588 B  (new)   +29268 B  (new)
async-load-reader-search-stream                                 +99176 B  (new)   +25151 B  (new)
async-load-signup-steps-plans-atomic-store                      +97777 B  (new)   +25487 B  (new)
async-load-layout-guided-tours-component                        +96558 B  (new)   +20551 B  (new)
async-load-lib-happychat-connection                             +64167 B  (new)   +17576 B  (new)
async-load-blocks-inline-help                                   +63659 B  (new)   +14930 B  (new)
async-load-reader-sidebar                                       +53789 B  (new)   +13512 B  (new)
async-load-my-sites-current-site-domain-warnings                +53656 B  (new)   +13250 B  (new)
async-load-blocks-support-article-dialog-dialog                 +50752 B  (new)   +15348 B  (new)
async-load-signup-steps-about                                   +47250 B  (new)   +12737 B  (new)
async-load-signup-steps-clone-credentials                       +45243 B  (new)   +11599 B  (new)
async-load-extensions-woocommerce-app-store-stats               +43947 B  (new)   +12235 B  (new)
async-load-blocks-product-purchase-features-list                +43588 B  (new)   +10868 B  (new)
async-load-extensions-woocommerce-app-store-stats-referrers     +43231 B  (new)   +11888 B  (new)
async-load-signup-steps-theme-selection                         +42137 B  (new)   +10394 B  (new)
async-load-blocks-reader-full-post                              +41680 B  (new)   +11160 B  (new)
async-load-my-sites-current-site-stale-cart-items-notice        +41049 B  (new)   +11474 B  (new)
async-load-signup-steps-rewind-form-creds                       +39755 B  (new)   +10426 B  (new)
async-load-layout-guided-tours                                  +37359 B  (new)   +10102 B  (new)
async-load-components-happychat                                 +35671 B  (new)   +11246 B  (new)
async-load-my-sites-current-site-notice                         +32894 B  (new)    +8097 B  (new)
async-load-signup-steps-import-url-onboarding                   +31398 B  (new)    +8994 B  (new)
async-load-signup-steps-site-style                              +30158 B  (new)    +9906 B  (new)
async-load-signup-steps-site-topic                              +28887 B  (new)    +8174 B  (new)
async-load-signup-steps-clone-ready                             +28087 B  (new)    +7732 B  (new)
async-load-quick-language-switcher                              +26205 B  (new)    +7462 B  (new)
async-load-reader-site-stream                                   +25120 B  (new)    +6849 B  (new)
async-load-reader-feed-stream                                   +25020 B  (new)    +6784 B  (new)
async-load-signup-steps-site                                    +25007 B  (new)    +6279 B  (new)
async-load-signup-steps-passwordless                            +24998 B  (new)    +6100 B  (new)
async-load-signup-steps-rewind-migrate                          +24902 B  (new)    +6901 B  (new)
async-load-components-jetpack-header-inmotion                   +24228 B  (new)    +7238 B  (new)
async-load-my-sites-guided-transfer                             +23610 B  (new)    +5647 B  (new)
async-load-signup-steps-creds-confirm                           +23436 B  (new)    +6427 B  (new)
async-load-signup-steps-creds-permission                        +23291 B  (new)    +6442 B  (new)
async-load-post-editor-editor-sharing-accordion                 +22964 B  (new)    +5817 B  (new)
async-load-layout-nps-survey-notice                             +22243 B  (new)    +5738 B  (new)
async-load-signup-steps-import-preview                          +21341 B  (new)    +6386 B  (new)
async-load-signup-steps-import-url                              +20716 B  (new)    +5790 B  (new)
async-load-signup-steps-site-or-domain                          +19287 B  (new)    +5472 B  (new)
async-load-components-jetpack-header-dreamhost                  +18513 B  (new)    +5595 B  (new)
async-load-post-editor-editor-author                            +18361 B  (new)    +5227 B  (new)
async-load-signup-steps-clone-destination                       +18290 B  (new)    +4991 B  (new)
async-load-components-jetpack-header-pressable                  +17386 B  (new)    +5324 B  (new)
async-load-signup-steps-survey                                  +17220 B  (new)    +4526 B  (new)
async-load-components-jetpack-header-liquidweb                  +16491 B  (new)    +5797 B  (new)
async-load-extensions-woocommerce-app-store-stats-listview      +16184 B  (new)    +4586 B  (new)
async-load-reader-tag-stream-main                               +16053 B  (new)    +4822 B  (new)
async-load-lib-preferences-helper                               +14672 B  (new)    +4816 B  (new)
async-load-signup-steps-clone-start                             +14210 B  (new)    +4115 B  (new)
async-load-signup-steps-clone-cloning                           +13716 B  (new)    +3987 B  (new)
async-load-signup-steps-rebrand-cities-welcome                  +13609 B  (new)    +3699 B  (new)
async-load-layout-community-translator-launcher                 +12296 B  (new)    +4398 B  (new)
async-load-signup-steps-reader-landing                          +12138 B  (new)    +3229 B  (new)
async-load-signup-steps-site-type                               +11894 B  (new)    +3230 B  (new)
async-load-components-jetpack-header-bluehost                   +11601 B  (new)    +3370 B  (new)
async-load-signup-steps-site-title                              +11466 B  (new)    +3110 B  (new)
async-load-blocks-jitm-templates-default                        +10788 B  (new)    +3069 B  (new)
async-load-reader-list-stream                                   +10638 B  (new)    +3057 B  (new)
async-load-signup-steps-clone-jetpack                           +10542 B  (new)    +2823 B  (new)
async-load-post-editor-editor-location                          +10040 B  (new)    +3021 B  (new)
async-load-components-jetpack-header-eurodns                     +9905 B  (new)    +2486 B  (new)
async-load-lib-abtest-test-helper                                +9191 B  (new)    +3111 B  (new)
async-load-signup-steps-site-picker                              +8925 B  (new)    +2588 B  (new)
async-load-signup-steps-rewind-were-backing                      +8804 B  (new)    +2586 B  (new)
async-load-signup-steps-creds-complete                           +8752 B  (new)    +2597 B  (new)
async-load-blocks-app-banner                                     +8626 B  (new)    +2881 B  (new)
async-load-components-community-translator                       +8272 B  (new)    +2600 B  (new)
async-load-signup-steps-test-step                                +8061 B  (new)    +2328 B  (new)
async-load-layout-masterbar-drafts-popover                       +7600 B  (new)    +2556 B  (new)
async-load-components-signup-site-preview-component              +7254 B  (new)    +2274 B  (new)
async-load-blocks-product-plan-overlap-notices                   +6477 B  (new)    +2091 B  (new)
async-load-post-editor-editor-seo-accordion                      +5888 B  (new)    +1967 B  (new)
async-load-docs-selectors                                        +5665 B  (new)    +1796 B  (new)
async-load-components-jetpack-header-woocommerce                 +5622 B  (new)    +1453 B  (new)
async-load-design-typography                                     +5527 B  (new)    +1690 B  (new)
async-load-post-editor-editor-post-formats-accordion             +5341 B  (new)    +1759 B  (new)
async-load-post-editor-editor-discussion                         +5053 B  (new)    +1595 B  (new)
async-load-blocks-inline-help-dialog                             +5008 B  (new)    +1974 B  (new)
async-load-blocks-jitm                                           +4890 B  (new)    +1735 B  (new)
async-load-reader-conversations-stream                           +4796 B  (new)    +1724 B  (new)
async-load-lib-network-connection                                +4214 B  (new)    +1562 B  (new)
async-load-blocks-login-social-connect-prompt                    +4103 B  (new)    +1405 B  (new)
async-load-featured-image                                        +2972 B  (new)    +1042 B  (new)
async-load-components-webpack-build-monitor                      +2591 B  (new)    +1132 B  (new)
async-load-blocks-jitm-templates-sidebar-banner                  +1036 B  (new)     +440 B  (new)
async-load-blocks-jitm-templates-notice                           +669 B  (new)     +404 B  (new)
async-load-components-jetpack-header-milesweb                     +640 B  (new)     +414 B  (new)
async-load-signup-steps-launch-site                               +548 B  (new)     +337 B  (new)
async-load-lib-rubberband-scroll-disable                          +494 B  (new)     +307 B  (new)
async-load-reader-team-main                                       +382 B  (new)     +270 B  (new)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Moment.js Locales (~108564 bytes added 📈 [gzipped])

name                    parsed_size         gzip_size
moment-locale-ru            +4545 B  (new)    +1606 B  (new)
moment-locale-mr            +3435 B  (new)    +1274 B  (new)
moment-locale-bo            +3248 B  (new)    +1073 B  (new)
moment-locale-uk            +3240 B  (new)    +1434 B  (new)
moment-locale-ta            +3024 B  (new)    +1164 B  (new)
moment-locale-be            +2947 B  (new)    +1289 B  (new)
moment-locale-cs            +2880 B  (new)    +1169 B  (new)
moment-locale-ka            +2754 B  (new)     +994 B  (new)
moment-locale-kn            +2753 B  (new)    +1149 B  (new)
moment-locale-ar            +2661 B  (new)    +1185 B  (new)
moment-locale-ne            +2575 B  (new)    +1108 B  (new)
moment-locale-sr-cyrl       +2516 B  (new)    +1090 B  (new)
moment-locale-ar-ly         +2504 B  (new)    +1111 B  (new)
moment-locale-gu            +2503 B  (new)    +1088 B  (new)
moment-locale-el            +2482 B  (new)    +1219 B  (new)
moment-locale-bn            +2471 B  (new)    +1051 B  (new)
moment-locale-sl            +2460 B  (new)     +992 B  (new)
moment-locale-pa-in         +2459 B  (new)    +1021 B  (new)
moment-locale-ml            +2403 B  (new)     +954 B  (new)
moment-locale-hi            +2369 B  (new)    +1076 B  (new)
moment-locale-ug-cn         +2364 B  (new)    +1019 B  (new)
moment-locale-km            +2320 B  (new)    +1009 B  (new)
moment-locale-lt            +2318 B  (new)    +1101 B  (new)
moment-locale-te            +2285 B  (new)     +959 B  (new)
moment-locale-es-us         +2207 B  (new)     +944 B  (new)
moment-locale-es-do         +2207 B  (new)     +945 B  (new)
moment-locale-es            +2190 B  (new)     +940 B  (new)
moment-locale-mn            +2183 B  (new)     +969 B  (new)
moment-locale-sk            +2148 B  (new)     +932 B  (new)
moment-locale-tlh           +2141 B  (new)     +890 B  (new)
moment-locale-my            +2138 B  (new)     +937 B  (new)
moment-locale-hy-am         +2135 B  (new)     +965 B  (new)
moment-locale-hr            +2123 B  (new)     +939 B  (new)
moment-locale-th            +2105 B  (new)     +859 B  (new)
moment-locale-ku            +2089 B  (new)     +975 B  (new)
moment-locale-pl            +2065 B  (new)     +995 B  (new)
moment-locale-si            +2058 B  (new)     +863 B  (new)
moment-locale-tg            +2051 B  (new)     +980 B  (new)
moment-locale-me            +2043 B  (new)     +956 B  (new)
moment-locale-lo            +2042 B  (new)     +811 B  (new)
moment-locale-fa            +2036 B  (new)     +939 B  (new)
moment-locale-sr            +2025 B  (new)     +952 B  (new)
moment-locale-hu            +2016 B  (new)     +931 B  (new)
moment-locale-nl-be         +2000 B  (new)     +939 B  (new)
moment-locale-nl            +1991 B  (new)     +936 B  (new)
moment-locale-fi            +1980 B  (new)     +894 B  (new)
moment-locale-bs            +1973 B  (new)     +877 B  (new)
moment-locale-gom-latn      +1965 B  (new)     +961 B  (new)
moment-locale-he            +1938 B  (new)     +878 B  (new)
moment-locale-ar-sa         +1930 B  (new)     +921 B  (new)
moment-locale-is            +1923 B  (new)     +896 B  (new)
moment-locale-lv            +1893 B  (new)     +848 B  (new)
moment-locale-ca            +1821 B  (new)     +856 B  (new)
moment-locale-tzm           +1819 B  (new)     +653 B  (new)
moment-locale-lb            +1810 B  (new)     +942 B  (new)
moment-locale-ky            +1771 B  (new)     +892 B  (new)
moment-locale-kk            +1760 B  (new)     +875 B  (new)
moment-locale-zh-cn         +1757 B  (new)     +921 B  (new)
moment-locale-az            +1756 B  (new)     +912 B  (new)
moment-locale-zh-tw         +1755 B  (new)     +906 B  (new)
moment-locale-zh-hk         +1750 B  (new)     +907 B  (new)
moment-locale-mk            +1748 B  (new)     +910 B  (new)
moment-locale-bg            +1728 B  (new)     +915 B  (new)
moment-locale-cv            +1691 B  (new)     +813 B  (new)
moment-locale-tzl           +1633 B  (new)     +867 B  (new)
moment-locale-dv            +1572 B  (new)     +818 B  (new)
moment-locale-br            +1546 B  (new)     +836 B  (new)
moment-locale-x-pseudo      +1531 B  (new)     +852 B  (new)
moment-locale-ja            +1531 B  (new)     +770 B  (new)
moment-locale-vi            +1530 B  (new)     +770 B  (new)
moment-locale-ss            +1517 B  (new)     +804 B  (new)
moment-locale-gl            +1517 B  (new)     +737 B  (new)
moment-locale-et            +1496 B  (new)     +767 B  (new)
moment-locale-tr            +1485 B  (new)     +799 B  (new)
moment-locale-ko            +1481 B  (new)     +733 B  (new)
moment-locale-mi            +1476 B  (new)     +748 B  (new)
moment-locale-de-at         +1469 B  (new)     +759 B  (new)
moment-locale-de-ch         +1460 B  (new)     +743 B  (new)
moment-locale-de            +1458 B  (new)     +752 B  (new)
moment-locale-ar-dz         +1449 B  (new)     +685 B  (new)
moment-locale-ar-tn         +1433 B  (new)     +679 B  (new)
moment-locale-ar-ma         +1430 B  (new)     +682 B  (new)
moment-locale-ar-kw         +1430 B  (new)     +680 B  (new)
moment-locale-uz            +1429 B  (new)     +709 B  (new)
moment-locale-cy            +1417 B  (new)     +760 B  (new)
moment-locale-jv            +1413 B  (new)     +737 B  (new)
moment-locale-fy            +1409 B  (new)     +769 B  (new)
moment-locale-yo            +1400 B  (new)     +704 B  (new)
moment-locale-fr            +1386 B  (new)     +726 B  (new)
moment-locale-gd            +1375 B  (new)     +754 B  (new)
moment-locale-eu            +1375 B  (new)     +691 B  (new)
moment-locale-fr-ch         +1371 B  (new)     +724 B  (new)
moment-locale-ms-my         +1370 B  (new)     +721 B  (new)
moment-locale-ms            +1361 B  (new)     +719 B  (new)
moment-locale-ur            +1355 B  (new)     +730 B  (new)
moment-locale-sd            +1354 B  (new)     +754 B  (new)
moment-locale-fr-ca         +1352 B  (new)     +712 B  (new)
moment-locale-id            +1348 B  (new)     +717 B  (new)
moment-locale-ga            +1346 B  (new)     +734 B  (new)
moment-locale-eo            +1337 B  (new)     +734 B  (new)
moment-locale-af            +1330 B  (new)     +727 B  (new)
moment-locale-se            +1325 B  (new)     +707 B  (new)
moment-locale-pt-br         +1314 B  (new)     +701 B  (new)
moment-locale-it-ch         +1310 B  (new)     +720 B  (new)
moment-locale-pt            +1305 B  (new)     +702 B  (new)
moment-locale-it            +1301 B  (new)     +717 B  (new)
moment-locale-sq            +1290 B  (new)     +701 B  (new)
moment-locale-sv            +1280 B  (new)     +698 B  (new)
moment-locale-tet           +1261 B  (new)     +687 B  (new)
moment-locale-en-nz         +1254 B  (new)     +686 B  (new)
moment-locale-en-au         +1254 B  (new)     +685 B  (new)
moment-locale-en-SG         +1250 B  (new)     +686 B  (new)
moment-locale-en-gb         +1250 B  (new)     +685 B  (new)
moment-locale-en-ie         +1249 B  (new)     +685 B  (new)
moment-locale-bm            +1244 B  (new)     +634 B  (new)
moment-locale-en-ca         +1238 B  (new)     +674 B  (new)
moment-locale-ro            +1223 B  (new)     +712 B  (new)
moment-locale-nb            +1223 B  (new)     +644 B  (new)
moment-locale-tl-ph         +1216 B  (new)     +657 B  (new)
moment-locale-en-il         +1215 B  (new)     +666 B  (new)
moment-locale-fo            +1203 B  (new)     +659 B  (new)
moment-locale-tzm-latn      +1187 B  (new)     +565 B  (new)
moment-locale-nn            +1184 B  (new)     +641 B  (new)
moment-locale-mt            +1173 B  (new)     +678 B  (new)
moment-locale-da            +1163 B  (new)     +632 B  (new)
moment-locale-sw            +1159 B  (new)     +632 B  (new)
moment-locale-uz-latn       +1155 B  (new)     +598 B  (new)

Locale data for moment.js. Unless you are upgrading the moment.js library, changes in these chunks are suspicious.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

@andrewserong andrewserong requested a review from a team January 8, 2020 02:47
@roo2
Copy link
Contributor

roo2 commented Jan 8, 2020

LGTM, it's a different issue but I just realized that it doesn't make sense to have the block settings menu available in gutenboarding either.

Screen Shot 2020-01-08 at 5 44 33 PM

Copy link
Member

@ramonjd ramonjd left a comment

Choose a reason for hiding this comment

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

LGTM

Would be good to get a 👍 from @Automattic/luna too to check for any gotchas

@andrewserong
Copy link
Member Author

andrewserong commented Jan 8, 2020

Thank you both for reviewing! @roo2 I was thinking the same thing, but thought I'd just keep the PR to the Next button, in case folks from Luna were testing out things in the sidebar / block settings?

@simison
Copy link
Member

simison commented Jan 8, 2020

it doesn't make sense to have the block settings menu available in gutenboarding either.

Yep, an issue here: #38686

<Link to={ next } className="gutenboarding__header-next-button" isPrimary isLarge>
{ NO__( 'Next' ) }
</Link>
{ ! isNextHidden && (
Copy link
Contributor

Choose a reason for hiding this comment

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

How about we change this to

Suggested change
{ ! isNextHidden && (
{ ! next && (

?

Falsey next can indicate that there's no next step to go to, and it'll allow us to drop the extra bool prop.

(I'm generally a bit wary of bool props controlling visibility of elements, since we might easily end up with an unwieldy number of them, especially in a context like Gutenboarding, which is all about a 'progressive chrome' that adds more and more UI elements as the user progresses through the steps of the onboarding block.)

Copy link
Member Author

Choose a reason for hiding this comment

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

@ockham thanks I wasn't too sure about adding the extra prop. In your suggestion, I think you mean we'd check that next is truthy to render the button?

The only problem is that checking if next is truthy will hide the disabled next button on the design step, too. Another way we could do it would be to set the value of next to a constant like 'HIDE_BUTTON' and if that's the value, then hide the button. What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

I implemented it that way at first in e548cae but then second guessed myself :)

Copy link
Member

@ramonjd ramonjd Jan 8, 2020

Choose a reason for hiding this comment

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

The only problem is that checking if next is truthy will hide the disabled next button on the design step, too.

I'm a bit unsure of myself, but I'll ask anyhow. Under the current architecture, wouldn't the design step define its own next in the switch block (thereby showing the next button)?

		case Step.IntentGathering:
			break;
        case Step. DesignSelection:
			next = Step. SomeFabulousStep;
			break;

Or is it more to do with the disabled state?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, I think it would. I'm possibly over thinking it, but I was mostly thinking about how do we want to handle the disabled state. So we've got 3 potential states for the button: hidden, disabled, and active link.

Copy link
Member

Choose a reason for hiding this comment

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

So we've got 3 potential states for the button: hidden, disabled, and active link.

Got it. I suppose we'll want to update the button based on validating input values in the main section or some such thing in the future.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ockham thanks I wasn't too sure about adding the extra prop. In your suggestion, I think you mean we'd check that next is truthy to render the button?

Yeah, you're right 😅

The only problem is that checking if next is truthy will hide the disabled next button on the design step, too.

That's a good point.

Another way we could do it would be to set the value of next to a constant like 'HIDE_BUTTON' and if that's the value, then hide the button. What do you think?

Let's avoid magic constants 😬 They're a bit too obscure, and go a bit against the whole types paradigm.


It's fine to go with either the current state of the PR (that checks next), or to revert to your original isNextHidden approach. My apologies for delaying this PR, we probably shouldn't overthink it at this stage.

Just to explain why I was skeptical about isNextHidden -- it's mostly about how this pattern will scale:

One of Gutenboarding's paradigms is a 'progressive chrome' -- the editor (with its header and possibly sidebar) adds more and more UI controls, as the user progresses through the onboarding steps in the onboarding block.

With the isSomethingHidden bool pattern, we'd end up with a number of bool props that control which of those UI elements to conditionally hide, depending on which step we're currently at. This is exacerbated by the fact that those UI controls might dynamically change (e.g. the 'Next' button label might be different at different stages).

I don't have a clear answer how to best solve this right now, but I'm leaning towards 'regions' components that accept a number of component props to be placed in different parts of the UI. See e.g. https://github.com/WordPress/gutenberg/blob/a8f43fa7e21a028b43d0cb023ac5c872a3fbd4d8/packages/edit-post/src/components/editor-regions/index.js (introduced by WordPress/gutenberg#18044, which has some explanation in its PR desc).

One problem that's a bit different for our use case is encapsulation -- I don't think we want gutenboarding.tsx to pass all those granular components down to e.g. the header or sidebar; instead, those 'container'/'region' components should infer themselves which components to display at what stage. Some of that information can be inferred from the onboarding data store; we'll still have to consider how to make those components aware of routing, tho (maybe we'll end up moving our main routing logic out of the onboarding block -- I don't know at this point). Note that this is basically a question about which pattern we'll use to build the 'custom editor'.

Anyway, I won't block this PR any longer 😅 Let's just keep this in mind as we add more 'conditional' UI elements.

@simison
Copy link
Member

simison commented Jan 9, 2020

Just realized we'll need to hide the button for now also at the "design picker" step and show it only at "page selector" step. That's the only step in the flow actually when the button is visible:

image
image

The label has to be "Create my site" at the last step.

(designs via paObgF-RB-p2 by @dwolfe )

Does that help thinking how to implement this?

@andrewserong
Copy link
Member Author

Thanks @simison I think it does! I should be able to go with @ockham's suggestion now that we can do away with the disabled button state. I'll take another look.

@andrewserong andrewserong requested a review from ramonjd January 10, 2020 03:43
@andrewserong
Copy link
Member Author

This is ready for another review — apologies if I've again over thought things! Because the button should only appear after a user has selected their design, I've gone ahead and moved the setSelectedDesign behaviour to the onboard store. (At the very least, this has been a useful exercise for me to get up to speed with the TypeScript types and how the data stores are structured).

@andrewserong andrewserong requested a review from ockham January 10, 2020 03:44
@andrewserong
Copy link
Member Author

Also, looks like the page layout picker will be moved to the onboard store in #38757, so hopefully this change should play nicely alongside that one, too.

Copy link
Member

@sirreal sirreal 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 working on this, much of the changes look good. The scope grew a bit beyond the original intention, but I think it's a reasonable size and set of changes 👍

I've left some comments. I do have concerns about putting more routing logic into the header, which spreads the routing logic out more. I'm not sure what the answer is. I'd like to keep the routing logic colocated, a dedicated module or a selector on the onboarding store seem like viable options. I'd like to yours and other's thoughts.

Comment on lines +23 to +31
const selectedDesign: Reducer<
import('@automattic/data-stores').VerticalsTemplates.Template | undefined,
ReturnType< typeof Actions[ 'setSelectedDesign' ] >
> = ( state = undefined, action ) => {
if ( action.type === ActionType.SET_SELECTED_DESIGN ) {
return action.selectedDesign;
}
return state;
};
Copy link
Member

Choose a reason for hiding this comment

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

👍 Looks good. Did you have any hangups working on this?

Comment on lines +13 to +25
export const setSelectedDesign = (
selectedDesign: import('@automattic/data-stores').VerticalsTemplates.Template | undefined
) => ( {
type: ActionType.SET_SELECTED_DESIGN as const,
selectedDesign,
} );
Copy link
Member

Choose a reason for hiding this comment

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

👍

client/landing/gutenboarding/steps.ts Outdated Show resolved Hide resolved
<Link to={ next } className="gutenboarding__header-next-button" isPrimary isLarge>
{ NO__( 'Next' ) }
</Link>
{ next && hasSelectedDesign && (
Copy link
Member

Choose a reason for hiding this comment

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

I don't like the idea of making the header more aware of the component and step logic and selecting more state. I'm not sure what the solution is right now 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

Me neither, at this point. I think we'll reconsider as Gutenboarding continues to evolve. Also see my (lengthy 😅 ) comment here: #38705 (comment)

@simison
Copy link
Member

simison commented Jan 10, 2020

I do have concerns about putting more routing logic into the header, which spreads the routing logic out more. I'm not sure what the answer is. I'd like to keep the routing logic colocated, a dedicated module or a selector on the onboarding store seem like viable options. I'd like to yours and other's thoughts.

The header is going to be evolving quite a bit in next weeks;

  • are we gonna have a back button or not?
  • is the next button gonna be in the header in future at all, or somewhere else, or both?
  • is the domain nudge in the header and when it becomes visible?

So I think it's safe to have some less-optimal code now in, and we'll just have to make sure to come back to improve it if we don't end up removing it.

I don't wanna undermine the work here by saying that this might be removed in future iterations, but it's kinda just the reality of Gutenboarding-prototype work currently; some things are more certain and some things less so.

Hope this helps!

@ramonjd
Copy link
Member

ramonjd commented Jan 12, 2020

are we gonna have a back button or not?

Sorry to bloat this PR's comment thread, it's probably best elaborate elsewhere, but for the record, we had some interesting debate about the role of a "synthetic" back button and alternatives back in the day.

At the core was the confusion brought about not only by duplicating the back button, but by the cases in which the back buttons (i.e., the browser's and ours) behaved differently, or did not do what the user expected them to do.

Towards the end, though no development tasks arose, an agreement percolated to the effect that any synthetic back button, or progress navigation we build into the flow should explicitly communicate to the user what is going to happen when they click on it.

See:

#34299 (comment)

Other related issues/PRs

#34598

#34390

@andrewserong andrewserong force-pushed the update/gutenboarding-hide-next-button-in-intent-gathering-step branch from cef3b85 to 39b9ba3 Compare January 12, 2020 23:40
@andrewserong
Copy link
Member Author

Thanks very much for the reviews and detailed thoughts everyone, it's very useful as I gradually form a mental model of what we're trying to achieve with Gutenboarding (and learn TypeScript in the process).

I've implemented the suggestions so far, and left the question of what to do about state-aware headers for another time. Let me know if there are any other changes you'd like, though!

So I think it's safe to have some less-optimal code now in, and we'll just have to make sure to come back to improve it if we don't end up removing it.

This sounds good to me — particularly while we're pre-launch with this flow, I'm more than happy to circle back on cleanup tasks as we go. Changes like in this PR are useful learning tools, so even if the code ends up being discarded it's still a good process to go through.

Thanks @ockham for going into greater detail about the "progressive chrome" idea and the scalability concerns of patterns in this part of the codebase. The regions idea does sound promising.

@sirreal I've worked through those suggested changes, thanks for the detailed look. Overall, working with TS is really growing on me, and having existing patterns and types in the codebase to refer to (like VerticalTemplates.Template) helped me get up to speed! I have a couple of other questions but I'll move them over to P2 because it's a bit more general TS :)

@andrewserong andrewserong requested a review from a team January 14, 2020 07:17
@andrewserong
Copy link
Member Author

If anyone gets a chance, this one's ready for another review.

Copy link
Contributor

@chriskmnds chriskmnds left a comment

Choose a reason for hiding this comment

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

Works as advertised. Just curious, if this being a next-step-trigger after selecting a design, should it also be available after selecting a design and collapsing the page selector? See:

Kapture 2020-01-14 at 11 33 29

Although I notice that going back/forth a step (to intent gathering), the design selection does not persist. So there may be some room to improve the flow there a little (?) e.g. keep the design selection throughout the session? If not, then the lingering selection when page-selector is collapsed currently doesn't serve much input (other than indicating what design was clicked previously).

@andrewserong andrewserong force-pushed the update/gutenboarding-hide-next-button-in-intent-gathering-step branch from 39b9ba3 to c27b14e Compare January 15, 2020 01:31
@andrewserong andrewserong force-pushed the update/gutenboarding-hide-next-button-in-intent-gathering-step branch from a3d690e to 1e3bc27 Compare January 15, 2020 03:23
@andrewserong
Copy link
Member Author

Thanks for reviewing @chriskmnds!

Just curious, if this being a next-step-trigger after selecting a design, should it also be available after selecting a design and collapsing the page selector?

That's a good question! Currently the Dialog component that's used to show the page layout selector is set to reset the selected design if a user hides the page selector. It'd be easy to remove this to change the behaviour and keep the user's selection, but we might then want a stronger indicator of which design is currently selected than just the focus state.

If not, then the lingering selection when page-selector is collapsed currently doesn't serve much input (other than indicating what design was clicked previously).

I think the lingering focus state on the selected design after hiding the page selector is probably fine. If you're using the keyboard to navigate, after hitting escape it means you don't lose your place in the list of designs.

I'll leave this for now to contain the scope of this PR, but it's a good point about holding on to the selected design state when a user navigates back to the intent gathering step. Let's circle back to this separately.

@andrewserong andrewserong merged commit 7a8934c into master Jan 15, 2020
@andrewserong andrewserong deleted the update/gutenboarding-hide-next-button-in-intent-gathering-step branch January 15, 2020 03:50
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jan 15, 2020
@dwolfe
Copy link

dwolfe commented Jan 15, 2020

Just one point of clarification: the design picker and the page selector are intended as sequential steps, so there's actually no "page selector collapsed" state. The only reason to collapse the page selector is to select a different design, which I indicated on the first iteration as a "Choose a different design" link.

@ramonjd ramonjd added [Goal] New Onboarding previously called Gutenboarding and removed [Goal] Gutenberg Working towards full integration with Gutenberg [Feature Group] Signup & Site Onboarding Tools for user registration and onboarding new users to the site. labels Jan 15, 2020
@andrewserong
Copy link
Member Author

Thanks for confirming @dwolfe!

@chriskmnds
Copy link
Contributor

chriskmnds commented Jan 16, 2020

I think the lingering focus state on the selected design after hiding the page selector is probably fine. If you're using the keyboard to navigate, after hitting escape it means you don't lose your place in the list of designs.

Makes sense, although I feel that's more like a secondary (or advanced) interaction. Seeing that blue box, my mind goes to "i have a design selected and pages within it (or some default empty). I can move on".

Quick thoughts:

  1. The design and page select steps are not clearly marked as separate to me. I kinda see that "expand/collapse" interaction as "expand to look inside". :D Also, the url is the same for both (gutenboarding/design) - may add more confusion? 🤔

  2. A "long list of designs" is probably something to be iterated on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Goal] New Onboarding previously called Gutenboarding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gutenboarding: hide "next" button in intent gathering step
9 participants