-
Notifications
You must be signed in to change notification settings - Fork 36
feat(core): Add autoStart feature #98
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: main
Are you sure you want to change the base?
Changes from all commits
3efdcf1
a30df69
187ca84
0498faf
e78150c
8c09f24
44ebbad
a445045
09ffe18
80eacbc
5d01989
6fdbc64
e7e1103
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,6 +36,8 @@ | |
| }, | ||
| "dependencies": { | ||
| "fast-equals": "^5.0.1", | ||
| "object-hash": "^3.0.0", | ||
| "react-native-mmkv-storage": "^0.9.1", | ||
| "react-native-responsive-dimensions": "^3.1.1", | ||
| "styled-components": "^5.3.9" | ||
| }, | ||
|
|
@@ -47,6 +49,7 @@ | |
| "@testing-library/react-native": "^11.5.4", | ||
| "@types/jest": "^29.4.1", | ||
| "@types/node": "^18.15.3", | ||
| "@types/object-hash": "^3.0.2", | ||
| "@types/react-test-renderer": "^18.0.0", | ||
| "@types/sinon": "^10.0.13", | ||
| "@types/styled-components": "^5.1.26", | ||
|
|
@@ -73,6 +76,7 @@ | |
| "peerDependencies": { | ||
| "react": ">=16.8.0", | ||
| "react-native": ">=0.50.0", | ||
| "react-native-mmkv-storage": ">=0.9.1", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to add this one to |
||
| "react-native-svg": ">=12.1.0" | ||
| }, | ||
| "peerDependenciesMeta": { | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,5 @@ | ||||||||||
| import { MMKVLoader } from "react-native-mmkv-storage"; | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is an optional dependency, so it's possible that it's not present in some environments. Shouldn't the import fail in those cases? A way to solve that is by using dynamic imports. import("react-native-mmkv-storage")
.then(({ MMKVLoader }) => {
// do something with the imported modules
})
.catch((error: unknown) => {
// handle whenever the module is not present
}); |
||||||||||
|
|
||||||||||
| const storage = new MMKVLoader().initialize(); | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we should initialize the store only when the |
||||||||||
|
|
||||||||||
| export default storage; | ||||||||||
|
Comment on lines
+3
to
+5
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should try to avoid default exports in favor of named exports 🙂
Suggested change
|
||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,13 @@ | ||
| import React, { forwardRef, useCallback, useImperativeHandle, useMemo, useRef, useState } from "react"; | ||
| import hash from "object-hash"; | ||
| import React, { forwardRef, useCallback, useImperativeHandle, useMemo, useRef, useState, useEffect } from "react"; | ||
| import { ColorValue, LayoutRectangle } from "react-native"; | ||
| import { useMMKVStorage } from "react-native-mmkv-storage"; | ||
|
|
||
| import { ChildFn, isChildFunction } from "../helpers/common"; | ||
| import storage from "../helpers/storage"; | ||
|
|
||
| import { | ||
| AutoStartOptions, | ||
| BackdropPressBehavior, | ||
| Motion, | ||
| OSConfig, | ||
|
|
@@ -18,6 +22,12 @@ import { | |
| import { TourOverlay, TourOverlayRef } from "./components/tour-overlay/TourOverlay.component"; | ||
|
|
||
| export interface SpotlightTourProviderProps { | ||
| /** | ||
| * Sets the default behaviour when the tour starts. | ||
| * | ||
| * @default never | ||
| */ | ||
| autoStart?: AutoStartOptions; // never - always - once | ||
| /** | ||
| * The children to render in the provider. It accepts either a React | ||
| * component, or a function that returns a React component. When the child is | ||
|
|
@@ -54,6 +64,12 @@ export interface SpotlightTourProviderProps { | |
| */ | ||
| onBackdropPress?: BackdropPressBehavior; | ||
| /** | ||
| * Handler which gets executed when {@link SpotlightTour.start|start} is | ||
| * invoked. It receives the {@link SpotlightTour.current|current} step index | ||
| * so you can access the current step where the tour starts. | ||
| */ | ||
| onStart?: () => void; | ||
alejo0o marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| /* | ||
| * Handler which gets executed when {@link SpotlightTour.stop|stop} is | ||
| * invoked. It receives the {@link StopParams} so | ||
| * you can access the `current` step index where the tour stopped | ||
|
|
@@ -92,6 +108,7 @@ export interface SpotlightTourProviderProps { | |
| */ | ||
| export const SpotlightTourProvider = forwardRef<SpotlightTour, SpotlightTourProviderProps>((props, ref) => { | ||
| const { | ||
| autoStart = "never", | ||
| children, | ||
| motion = "bounce", | ||
| nativeDriver = true, | ||
|
|
@@ -101,10 +118,12 @@ export const SpotlightTourProvider = forwardRef<SpotlightTour, SpotlightTourProv | |
| overlayOpacity = 0.45, | ||
| spotPadding = 16, | ||
| steps, | ||
| onStart, | ||
| } = props; | ||
|
|
||
| const [current, setCurrent] = useState<number>(); | ||
| const [spot, setSpot] = useState(ZERO_SPOT); | ||
| const [tourId, setTourId] = useMMKVStorage("tourId", storage, ""); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
| const overlay = useRef<TourOverlayRef>({ | ||
| hideTooltip: () => Promise.resolve({ finished: false }), | ||
|
|
@@ -118,7 +137,7 @@ export const SpotlightTourProvider = forwardRef<SpotlightTour, SpotlightTourProv | |
| overlay.current.hideTooltip(), | ||
| Promise.resolve().then(step.before), | ||
| ]) | ||
| .then(() => setCurrent(index)); | ||
| .then(() => setCurrent(index)); | ||
alejo0o marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| }, [steps]); | ||
|
|
||
|
|
@@ -128,7 +147,24 @@ export const SpotlightTourProvider = forwardRef<SpotlightTour, SpotlightTourProv | |
|
|
||
| const start = useCallback((): void => { | ||
| renderStep(0); | ||
| }, [renderStep]); | ||
| onStart?.(); | ||
| }, [renderStep, onStart]); | ||
|
|
||
| const startOnce = useCallback(() => { | ||
| if (!tourId) { | ||
| setTourId(hash(steps)); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens if the user has multiple tour providers in their app. I think we will need them to provide an ID of the tour to use this feature, and the value we check against is the |
||
| renderStep(0); | ||
| onStart?.(); | ||
|
Comment on lines
+156
to
+157
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think these two can be replaced by calling the |
||
| } | ||
| }, [renderStep, onStart, steps]); | ||
|
|
||
| useEffect(() => { | ||
| if (autoStart === "always") { | ||
| start(); | ||
| } else if (autoStart === "once") { | ||
| startOnce(); | ||
| } | ||
alejo0o marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| }, [renderStep, autoStart]); | ||
|
|
||
| const stop = useCallback((): void => { | ||
| setCurrent(prev => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,7 @@ | ||
| /* eslint-disable max-classes-per-file */ | ||
| import { useState } from "react"; | ||
| import { Animated, LayoutRectangle } from "react-native"; | ||
| import { MMKVInstance } from "react-native-mmkv-storage"; | ||
|
|
||
| import { | ||
| isAnimatedTimingInterpolation, | ||
|
|
@@ -136,7 +138,18 @@ jest | |
| timing: timingMock, | ||
| }, | ||
| }; | ||
| }); | ||
| }) | ||
| /* eslint-disable @typescript-eslint/no-unused-vars */ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need this? 🤔 |
||
| .mock("react-native-mmkv-storage", () => ({ | ||
| MMKVLoader: jest.fn().mockImplementation(() => ({ | ||
| initialize: () => jest.fn(), | ||
| })), | ||
| useMMKVStorage: (_key: string, _storage: MMKVInstance, defaultValue: string) => { | ||
| const [value, setValue] = useState(defaultValue); | ||
| const setMockValue = (newValue: string): void => setValue(newValue); | ||
| return [value, setMockValue]; | ||
| }, | ||
| })); | ||
|
|
||
| afterEach(() => { | ||
| jest.resetAllMocks(); | ||
|
|
||
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.
Let's move this to devDependencies as it's an optional peer dependency 🙂