-
Notifications
You must be signed in to change notification settings - Fork 330
Enhancement/#11812 - Implement new designs for the TourTooltip component
#11934
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
base: develop
Are you sure you want to change the base?
Conversation
Adjust close icon size and conditionally render indicators based on feature availability.
…omponent. Adjust tooltip display based on setup flow refresh feature availability.
…ant. Enhance decorators for fetchMock setup to improve story switching functionality.
|
Storybook is ready:
|
|
Size Change: +2.89 kB (+0.13%) Total Size: 2.23 MB ℹ️ View Unchanged
|
|
Build files for 5888b8e are ready:
|
nfmohit
left a comment
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.
Thank you for your work on this, @hussain-t.
I've left a number of comments for your consideration. Please let me know if you have any questions or concerns. Thanks!
assets/sass/components/tour-tooltip/_googlesitekit-tour-tooltip.scss
Outdated
Show resolved
Hide resolved
assets/sass/components/tour-tooltip/_googlesitekit-tour-tooltip.scss
Outdated
Show resolved
Hide resolved
…tiary style based on setupFlowRefreshEnabled state.
…ng adjustments to title and body properties, and add conditional styles for legacy design.
… for title and content based on setupFlowRefreshEnabled state.
…um, removing conditional logic based on setupFlowRefreshEnabled state.
…sting button margin for improved layout.
…ing max-width and padding for improved layout and specificity.
Thanks for your kind review, @nfmohit! I've addressed most of the feedback. Please take another look. |
nfmohit
left a comment
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.
Thank you for addressing my comments, @hussain-t!
I've left a few additional comments for your consideration. Please let me know if you have any questions or concerns. Thanks!
| top: $grid-gap-phone; | ||
| } | ||
|
|
||
| .googlesitekit-tooltip-button { |
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.
| top: $grid-gap-phone; | ||
| } | ||
|
|
||
| .googlesitekit-tooltip-button { |
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.
It looks like we're also missing hover and focus styles for both the primary and tertiary buttons.
We should clarify them with Sigal (if necessary) and add them.
|
|
||
| .googlesitekit-tooltip-button { | ||
| border-radius: $br-lg; | ||
| line-height: 20px; |
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.



Summary
Addresses issue:
TourTooltipcomponent to reflect the new design. #11812Relevant 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