Tour Kit: Update spotlight effect, other fixes/cleanup #58854

chriskmnds · 2021-12-06T13:41:53Z

Changes proposed in this Pull Request

Updates spotlight effect
- now being reference element aware (wraps the entire reference by default)
- customizable (can be overwritten with custom CSS props e.g. in Welcome Tour)
Updates Storybook examples
- capture placement on all sides
- resizable container for fiddling with scrolling etc.
Other fixes
- issue with the arrow tip indicator
- issue with Popper misplacing the step on mount/ initial render

Media

Default spotlight:

Customized for Welcome Tour (fixed-size spotlight, some border radius):

Testing instructions

Checkout branch, then yarn dev --sync from apps/editing-toolkit to sync the welcome tour to the sandbox
yarn run tour-kit:storybook:start in another tab for Storybook demos
Welcome Tour works as expected with/out ?welcome-tour-next
Stories work as expected

Closes #58642, #58748
Related to #58644

github-actions · 2021-12-06T13:42:12Z

Link to Calypso live: https://calypso.live?image=registry.a8c.com/calypso/app:build-24285
Link to Jetpack Cloud live: https://calypso.live?image=registry.a8c.com/calypso/app:build-24285&env=jetpack

matticbot · 2021-12-06T13:43:05Z

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

chriskmnds · 2021-12-06T13:51:49Z

packages/tour-kit/src/components/tour-kit-frame.tsx

+	useEffect( () => {
+		/*
+		 * Fixes issue with Popper misplacing the instance on mount
+		 * See: https://stackoverflow.com/questions/65585859/react-popper-incorrect-position-on-mount
+		 */
+		update && update();
+		setTimeout( () => setTourReady( true ) );
+	}, [ update ] );


Bug:

Fix without tourReady (brief flicker, not always visible):

(wish there was a better way) 😐

I made a test with the Welcome Tour and it seems that the "flickering" is still there: https://d.pr/v/24r3Xb (look when you arrive at the third point)

Thanks! Yep, see it.

I think I've found a better fix 🤞 Take another look? :-)

noticing this again while testing. looking

Alright. Hopefully fixed now (albeit other dragons may lurk 😬 ). Give it a few more tests? :-)

This is what we should expect:

Ok, now it seems to be working, well done!

chriskmnds · 2021-12-06T14:07:09Z

packages/tour-kit/src/components/tour-kit-frame.tsx

 	const tourContainerRef = useRef( null );
-	const popperElementRef = useRef( null );
+	const [ popperElement, sePopperElement ] = useState< HTMLElement | null >( null );


Recommended way for setting the popper element (useState triggers a re-render): https://popper.js.org/react-popper/v2/

Nice one 💯

escapemanuele

Looks very good to me, great job! 💪
Left a comment for a problem with setTourReady

escapemanuele · 2021-12-06T15:52:51Z

packages/tour-kit/src/components/tour-kit-frame.tsx

 	const tourContainerRef = useRef( null );
-	const popperElementRef = useRef( null );
+	const [ popperElement, sePopperElement ] = useState< HTMLElement | null >( null );


Nice one 💯

escapemanuele · 2021-12-07T17:36:46Z

packages/tour-kit/src/components/tour-kit-frame.tsx

+	useEffect( () => {
+		/*
+		 * Fixes issue with Popper misplacing the instance on mount
+		 * See: https://stackoverflow.com/questions/65585859/react-popper-incorrect-position-on-mount
+		 */
+		update && update();
+		setTimeout( () => setTourReady( true ) );
+	}, [ update ] );


I made a test with the Welcome Tour and it seems that the "flickering" is still there: https://d.pr/v/24r3Xb (look when you arrive at the third point)

matticbot · 2021-12-08T07:54:43Z

This PR modifies the release build for editing-toolkit

To test your changes on WordPress.com, run install-plugin.sh editing-toolkit update/tour-kit-spotlight on your sandbox.

To deploy your changes after merging, see the documentation: PCYsg-mMA-p2

escapemanuele

LGTM! After tests pass =>

tyxla · 2022-01-11T12:34:35Z

packages/tour-kit/src/components/tour-kit-spotlight.tsx

 	const clipRepositionProps = referenceElement
 		? {
-				style: popperStyles?.popper,
+				style: {
+					...( clipDimensions && clipDimensions ),


Do we really need the clipDimensions && check or can we just do...clipDimensions? ...null should be fine.

Thanks @tyxla. Added follow-up issue. 🙇

(we are tracking in internal PT)

Thanks, Christos 🙌

tyxla · 2022-01-11T12:34:55Z

packages/tour-kit/src/components/tour-kit-spotlight.tsx

+				style: {
+					...( clipDimensions && clipDimensions ),
+					...popperStyles?.popper,
+					...( styles && styles ),


Same for styles as clipDimensions above.

chriskmnds added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. Welcome Tour Tour Kit labels Dec 6, 2021

chriskmnds added this to the Editor welcome tour NEXT milestone Dec 6, 2021

chriskmnds requested a review from escapemanuele December 6, 2021 13:41

chriskmnds self-assigned this Dec 6, 2021

chriskmnds commented Dec 6, 2021

View reviewed changes

escapemanuele reviewed Dec 7, 2021

View reviewed changes

escapemanuele approved these changes Dec 8, 2021

View reviewed changes

chriskmnds force-pushed the update/tour-kit-spotlight branch from 8369926 to ab099dc Compare December 9, 2021 11:47

chriskmnds force-pushed the update/tour-kit-spotlight branch from ab099dc to 9212375 Compare January 10, 2022 10:40

chriskmnds added 12 commits January 10, 2022 13:10

take 1, start

96d11e4

reference aware clip dims. spotlight style override

f7c79ae

cleanup

20aafc5

remove custom popper hook. placement auto for step frame

d412320

fix initial popper misplacement and flicker. update stories.

c6aef2d

cleanup

0b82c4f

cleanup. spotlight readme updates

f631c69

fix flicker

89d5512

stories. resizable references

7f51ef7

spotlight. fix placement overflow when reference scrolled out of view

b77633a

fix flicker. skip positioning and hide until ready

c1b9393

cleanup

51323f6

chriskmnds force-pushed the update/tour-kit-spotlight branch from 9212375 to 51323f6 Compare January 10, 2022 11:11

chriskmnds merged commit 4a3ab53 into trunk Jan 10, 2022

chriskmnds deleted the update/tour-kit-spotlight branch January 10, 2022 11:24

github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jan 10, 2022

chriskmnds mentioned this pull request Jan 10, 2022

Tour Kit: Cleanup - update CSS --visible modifier to Calypso convention #59905

Closed

4 tasks

chriskmnds modified the milestones: Editor welcome tour NEXT, Tour Kit: Foundations Jan 11, 2022

tyxla reviewed Jan 11, 2022

View reviewed changes

chriskmnds mentioned this pull request Jan 12, 2022

Tour Kit: Cleanup - refactor Popper clipDimensions/styles checks #59981

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tour Kit: Update spotlight effect, other fixes/cleanup #58854

Tour Kit: Update spotlight effect, other fixes/cleanup #58854

chriskmnds commented Dec 6, 2021 •

edited

Loading

github-actions bot commented Dec 6, 2021 •

edited by matticbot

Loading

matticbot commented Dec 6, 2021

chriskmnds Dec 6, 2021 •

edited

Loading

escapemanuele Dec 7, 2021

chriskmnds Dec 8, 2021

chriskmnds Dec 8, 2021

chriskmnds Dec 8, 2021

escapemanuele Dec 8, 2021

chriskmnds Dec 6, 2021 •

edited

Loading

escapemanuele Dec 6, 2021

escapemanuele left a comment

escapemanuele Dec 6, 2021

escapemanuele Dec 7, 2021

matticbot commented Dec 8, 2021

escapemanuele left a comment •

edited

Loading

tyxla Jan 11, 2022

chriskmnds Jan 12, 2022 •

edited

Loading

tyxla Jan 12, 2022

tyxla Jan 11, 2022

Tour Kit: Update spotlight effect, other fixes/cleanup #58854

Tour Kit: Update spotlight effect, other fixes/cleanup #58854

Conversation

chriskmnds commented Dec 6, 2021 • edited Loading

Changes proposed in this Pull Request

Media

Testing instructions

github-actions bot commented Dec 6, 2021 • edited by matticbot Loading

matticbot commented Dec 6, 2021

chriskmnds Dec 6, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chriskmnds Dec 6, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

escapemanuele left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matticbot commented Dec 8, 2021

escapemanuele left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chriskmnds Jan 12, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chriskmnds commented Dec 6, 2021 •

edited

Loading

github-actions bot commented Dec 6, 2021 •

edited by matticbot

Loading

chriskmnds Dec 6, 2021 •

edited

Loading

chriskmnds Dec 6, 2021 •

edited

Loading

escapemanuele left a comment •

edited

Loading

chriskmnds Jan 12, 2022 •

edited

Loading