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

fix: SpotlightPopoverTour bugs #1826

Merged
merged 18 commits into from
Nov 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .changeset/three-glasses-complain.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@razorpay/blade": patch
---

fix(blade): fixed SpotlightPopoverTour bugs

- Safari body-scroll-lock causing the page to get clipped because storybook doesn't set width/height on body - fixed by setting width/height
- Initial delay of opening the mask - fixed by immediately updating the mask size on initial render
- Delay of transitioning between steps which occurs because we need to wait for the animation to finish before scrolling otherwise the scroll gets interrupted - fixed by reduced this to 100ms
5 changes: 5 additions & 0 deletions packages/blade/.storybook/react/global.css
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,8 @@
/* Need this to ensure mdx stories don't break visually */
box-sizing: border-box;
}

body {
width: 100%;
height: 100%;
}
14 changes: 14 additions & 0 deletions packages/blade/docs/guides/Usage.stories.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,20 @@ function AppWrapper(): JSX.Element {
export default AppWrapper;
```

## iOS Safari Specific Setup
Copy link
Collaborator

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?


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

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?

Copy link
Member Author

@anuraghazra anuraghazra Nov 28, 2023

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


```css
body {
width: 100%;
height: 100%;
}
```

## Mapping Components from Figma to Blade in your code

Blade is built with **"What you see in Figma is what you get on Code" ** philosophy.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import { Link } from '~components/Link';
import { Sandbox } from '~utils/storybook/Sandbox';
import StoryPageWrapper from '~utils/storybook/StoryPageWrapper';
import { isReactNative } from '~utils';
import { SandboxHighlighter } from '~utils/storybook/Sandbox/SandpackEditor';

const Page = (): React.ReactElement => {
return (
Expand Down Expand Up @@ -134,6 +135,20 @@ const Page = (): React.ReactElement => {
export default App;
`}
</Sandbox>
<Title>iOS Safari Specific Setup</Title>
<Text marginTop="spacing.4">
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.
</Text>
<SandboxHighlighter showLineNumbers={false} theme="light">
{`
body {
width: 100%;
height: 100%;
}
`}
Comment on lines +145 to +150
Copy link
Member

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

</SandboxHighlighter>
</StoryPageWrapper>
);
};
Expand Down Expand Up @@ -225,6 +240,51 @@ const BottomSheetTemplate: ComponentStory<typeof BottomSheetComponent> = ({ ...a
return (
<BaseBox>
<Button onClick={() => setIsOpen(true)}>{isOpen ? 'close' : 'open'}</Button>
<Text marginY="spacing.11">
Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has
been the industry's standard dummy text ever since the 1500s, when an unknown printer took a
galley of type and scrambled it to make a type specimen book. It has survived not only five
centuries, but also the leap into electronic typesetting, remaining essentially unchanged.
It was popularised in the 1960s with the release of Letraset sheets containing Lorem Ipsum
passages, and more recently with desktop publishing software like Aldus PageMaker including
versions of Lorem Ipsum.
</Text>
<Text marginY="spacing.11">
Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has
been the industry's standard dummy text ever since the 1500s, when an unknown printer took a
galley of type and scrambled it to make a type specimen book. It has survived not only five
centuries, but also the leap into electronic typesetting, remaining essentially unchanged.
It was popularised in the 1960s with the release of Letraset sheets containing Lorem Ipsum
passages, and more recently with desktop publishing software like Aldus PageMaker including
versions of Lorem Ipsum.
</Text>
<Text marginY="spacing.11">
Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has
been the industry's standard dummy text ever since the 1500s, when an unknown printer took a
galley of type and scrambled it to make a type specimen book. It has survived not only five
centuries, but also the leap into electronic typesetting, remaining essentially unchanged.
It was popularised in the 1960s with the release of Letraset sheets containing Lorem Ipsum
passages, and more recently with desktop publishing software like Aldus PageMaker including
versions of Lorem Ipsum.
</Text>
<Text marginY="spacing.11">
Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has
been the industry's standard dummy text ever since the 1500s, when an unknown printer took a
galley of type and scrambled it to make a type specimen book. It has survived not only five
centuries, but also the leap into electronic typesetting, remaining essentially unchanged.
It was popularised in the 1960s with the release of Letraset sheets containing Lorem Ipsum
passages, and more recently with desktop publishing software like Aldus PageMaker including
versions of Lorem Ipsum.
</Text>
<Text marginY="spacing.11">
Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has
been the industry's standard dummy text ever since the 1500s, when an unknown printer took a
galley of type and scrambled it to make a type specimen book. It has survived not only five
centuries, but also the leap into electronic typesetting, remaining essentially unchanged.
It was popularised in the 1960s with the release of Letraset sheets containing Lorem Ipsum
passages, and more recently with desktop publishing software like Aldus PageMaker including
versions of Lorem Ipsum.
</Text>
<BottomSheetComponent
{...args}
isOpen={isOpen}
Expand Down
63 changes: 41 additions & 22 deletions packages/blade/src/components/SpotlightPopoverTour/Tour.web.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,11 @@ const SpotlightPopoverTour = ({
});

// delayed state is used to let the transition finish before reacting to the state changes
const delayedActiveStep = useDelayedState(activeStep, transitionDelay);
const delayedSize = useDelayedState(size, transitionDelay);
const [delayedActiveStep] = useDelayedState(activeStep, transitionDelay);
const [delayedSize, setDelayedSize] = useDelayedState(size, transitionDelay);
// keep track of when we are transitioning between steps
const isTransitioning = useIsTransitioningBetweenSteps(activeStep, transitionDelay);
const [isScrolling, setIsScrolling] = useState(false);

const currentStepRef = refIdMap.get(steps[activeStep]?.name);
const intersection = useIntersectionObserver(currentStepRef!, {
Expand Down Expand Up @@ -96,18 +97,29 @@ const SpotlightPopoverTour = ({
});
}, []);

const updateMaskSize = useCallback(() => {
const ref = refIdMap.get(steps[activeStep]?.name);
if (!ref?.current) return;
const updateMaskSize = useCallback(
(shouldSkipDelay = false) => {
const ref = refIdMap.get(steps[activeStep]?.name);
if (!ref?.current) return;

const rect = ref.current.getBoundingClientRect();
setSize({
x: rect.x,
y: rect.y,
width: rect.width,
height: rect.height,
});
}, [activeStep, refIdMap, steps]);
const rect = ref.current.getBoundingClientRect();
setSize({
x: rect.x,
y: rect.y,
width: rect.width,
height: rect.height,
});
if (shouldSkipDelay) {
setDelayedSize({
x: rect.x,
y: rect.y,
width: rect.width,
height: rect.height,
});
}
},
[activeStep, refIdMap, setDelayedSize, steps],
);

const scrollToStep = useCallback(() => {
const ref = refIdMap.get(steps[delayedActiveStep]?.name);
Expand All @@ -116,39 +128,44 @@ const SpotlightPopoverTour = ({
// If the element is already in view, don't scroll
if (intersection?.isIntersecting) return;

setIsScrolling(true);
smoothScroll(ref.current, {
behavior: 'smooth',
block: 'center',
inline: 'center',
})
.then(() => {
updateMaskSize();
// wait for the scroll to finish before updating the mask size
// We also don't want to delay the size update since its already delayed by the scroll
updateMaskSize(true);
})
.finally(() => {
// do nothing
setIsScrolling(false);
});
}, [delayedActiveStep, refIdMap, steps, updateMaskSize, intersection?.isIntersecting]);

// Update the size of the mask when the active step changes
useIsomorphicLayoutEffect(() => {
updateMaskSize();
}, [isOpen, activeStep, refIdMap, steps, updateMaskSize]);
}, [activeStep, updateMaskSize]);

// Scroll into view when the active step changes
useIsomorphicLayoutEffect(() => {
// We need to wait for the transition to finish before scrolling
// Otherwise the browser sometimes interrupts the scroll
const scrollDelay = 100;
setTimeout(() => {
if (!isOpen) return;
if (isTransitioning) return;
scrollToStep();
}, transitionDelay);
}, scrollDelay);
}, [isOpen, scrollToStep, isTransitioning]);

useLockBodyScroll(isOpen);

// reset the mask size when the tour is closed
React.useEffect(() => {
useIsomorphicLayoutEffect(() => {
if (isOpen) {
updateMaskSize();
// on initial mount, we don't want to delay the size update
updateMaskSize(true);
onOpenChange?.({ isOpen });
}
if (!isOpen) {
Expand All @@ -162,6 +179,8 @@ const SpotlightPopoverTour = ({
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [isOpen]);

useLockBodyScroll(isOpen);

const contextValue = useMemo(() => {
return { attachStep, removeStep };
}, [attachStep, removeStep]);
Expand All @@ -171,7 +190,7 @@ const SpotlightPopoverTour = ({
<FloatingPortal>
{isOpen ? (
<SpotlightPopoverTourMask
isTransitioning={isTransitioning}
isTransitioning={isTransitioning || isScrolling}
padding={theme.spacing[4]}
size={delayedSize}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
useTransitionStyles,
autoUpdate,
useClick,
useDismiss,
FloatingFocusManager,
} from '@floating-ui/react';
import React from 'react';
Expand Down Expand Up @@ -108,10 +107,9 @@ const TourPopover = ({
// remove click handler if popover is controlled
const isControlled = isOpen !== undefined;
const click = useClick(context, { enabled: !isControlled });
const dismiss = useDismiss(context);
const role = useRole(context);

const { getFloatingProps } = useInteractions([click, dismiss, role]);
const { getFloatingProps } = useInteractions([click, role]);

const contextValue = React.useMemo(() => {
return {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { composeStories } from '@storybook/react';
import * as tourStories from './Tour.stories';
import * as tourStories from './docs/Tour.stories';
import { Box } from '~components/Box';
import { Heading } from '~components/Typography';

const allStories = Object.values(composeStories(tourStories));

export const Tour = (): JSX.Element => {
export const SpotlightPopoverTour = (): JSX.Element => {
return (
<Box display="flex" flexDirection="column" gap="spacing.4">
{allStories.map((Story) => {
Expand All @@ -21,8 +21,8 @@ export const Tour = (): JSX.Element => {
};

export default {
title: 'Components/KitchenSink/Tour',
component: Tour,
title: 'Components/KitchenSink/SpotlightPopoverTour',
component: SpotlightPopoverTour,
parameters: {
// enable Chromatic's snapshotting only for kitchensink
chromatic: { disableSnapshot: false },
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { Meta } from '@storybook/addon-docs';
import { TourDocs } from './TourDocs.stories';

<Meta title="Components/SpotlightPopoverTour/Docs" />

<TourDocs />
Loading
Loading