-
Notifications
You must be signed in to change notification settings - Fork 147
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
fix: SpotlightPopoverTour bugs #1826
Conversation
🦋 Changeset detectedLatest commit: 3522f22 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
BundleMonFiles updated (1)
Unchanged files (13)
Total files change +303B +0.05% Final result: ✅ View report in BundleMon website ➡️ |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 3522f22:
|
✅ PR title follows Conventional Commits specification. |
@anuraghazra when i close the tour say at 2/3 then it shall restart right from 1/3? Right now on storybook it resumes from 2/3 |
@@ -45,6 +45,20 @@ function AppWrapper(): JSX.Element { | |||
export default AppWrapper; | |||
``` | |||
|
|||
## iOS Safari Specific Setup |
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.
can you add this to Bottmsheet and Popover's docs as well in on top of the page something that is highlighted?
Tour is a controlled component, so this behaviour is entirely upon the product usecase. We just give them the ability to change the |
Not able to reproduce this with iOS simulator can you provide a video? |
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.
Looks good to me. Added the exit animation delay thing in slack thread
When using BottomSheet or SpotlightPopoverTour, | ||
Make sure to set a width/height to the `body` otherwise when they open, the page will get clipped. | ||
|
||
This happens due to a bug in iOS safari where it won't compute the height of the body correctly. |
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.
is there any link to this issue that we can add here?
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.
I observed this weird bug on computed properties of safari dev tools, but don't have exact webkit bug reference.
There are multiple issues with safari's height calculations:
https://developer.apple.com/forums/thread/128949
https://stackoverflow.com/questions/68705153/height-100-stopped-working-when-run-in-safari
chakra-ui/chakra-ui#6027
@@ -28,7 +28,7 @@ export const parameters = { | |||
}, | |||
// on development setting it to undefined so that on 'live reload' it won't switch | |||
// to docs panel while developing the component | |||
viewMode: process.env.NODE_ENV === 'development' ? undefined : 'docs', | |||
viewMode: process.env.NODE_ENV === 'development' ? 'docs' : '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.
condition ? docs : 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.
Oops.
{` | ||
body { | ||
width: 100%; | ||
height: 100%; | ||
} | ||
`} |
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.
nit: This won't get highlighted since in SandboxHighlighter I had kept .ts
file as default. Might have to add option to set .css
file there
Fixes 3 bugs:
body
- fixed by setting width/height