-
Notifications
You must be signed in to change notification settings - Fork 909
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
402 implement site kit setup flow #21978
base: trunk
Are you sure you want to change the base?
Conversation
add stepper to ui library export stepper fix stories args move margin out of the plugin fix story space
add translation to the stepper
Pull Request Test Coverage Report for Build 6071ff29b1e0586bcd7e4134fefdbe4fe384af78Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CR 🏗️
- About the changelogs/label: You put enhancement while I don't think the unreleased should go in there, right? So probably you meant to signal
enhancement
for the UI library one. You can do both, see documentation.
AT ❓
- Getting the following React warning (first run, but already on just not connected):
Warning: Failed prop type: The prop
isComplete
is marked as required inStepper
, but its value isundefined
.
-
The current step svg does not center in the stepper. In the first it is off to the left/bottom? And the rest is off to the right/bottom?
-
No animations? The FTC' vertical stepper has one -- using our ProgressBar that would be fixed
-
Something seems off with the check size, maybe just the outline thing
-
The UI library storybook story is not responding to the inputs
-
The dropdown menu does not show any hover/selection effect
src/dashboard/application/configuration/dashboard-configuration.php
Outdated
Show resolved
Hide resolved
src/dashboard/application/configuration/dashboard-configuration.php
Outdated
Show resolved
Hide resolved
packages/js/src/dashboard/components/google-site-kit-connection-widget.js
Outdated
Show resolved
Hide resolved
packages/js/src/dashboard/components/google-site-kit-connection-widget.js
Outdated
Show resolved
Hide resolved
packages/js/src/dashboard/components/google-site-kit-connection-widget.js
Outdated
Show resolved
Hide resolved
packages/js/src/dashboard/components/google-site-kit-connection-widget.js
Outdated
Show resolved
Hide resolved
Changed the steps to string, and cached the complete state
camel case in js
A merge conflict has been detected for the proposed code changes in this PR. Please resolve the conflict by either rebasing the PR or merging in changes from the base branch. |
fix docs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CR ✅ especially if you fix the typo 😉
- I like your new status array! Makes it clear what is happening. Also makes me spot a potential shared hook there, but lets not 😁
- Typo detected
- Will you add a follow-up for the dropdown menu storybook too? 11b7cc1#r1940659740
- Sorry to mention here, but I think we might need a follow-up for the arrow -- presuming we want consistency (@uxkai ?):
- I would prefer if the UI library changelog is an entry per component. They are not otherwise connected
packages/ui-library/src/components/dropdown-menu/docs/component.md
Outdated
Show resolved
Hide resolved
…t.md Co-authored-by: Igor <[email protected]>
Context
Summary
This PR can be summarized in the following changelog entry:
Relevant technical choices:
Test instructions
Test instructions for the acceptance test before the PR gets merged
This PR can be acceptance tested by following these steps:
UI library - Stepper component
UI library - DropdownMenu component
Relevant test scenarios
Test instructions for QA when the code is in the RC
QA can test this PR by following these steps:
Impact check
This PR affects the following parts of the plugin, which may require extra testing:
UI changes
Other environments
[shopify-seo]
, added test instructions for Shopify and attached theShopify
label to this PR.Documentation
Quality assurance
Innovation
innovation
label.Fixes Implement Site Kit setup flow without the consent modal