Skip to content

Navigator: add basic location history #37416

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.

Sign up for GitHub

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

Merged
ciampo merged 9 commits into from
Jan 21, 2022
Merged

Navigator: add basic location history #37416

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c49be73
`Navigator`: add basic location history
ciampo Dec 15, 2021
6dcb1af
Refactor `Navigator` usage across repository
ciampo Dec 15, 2021
fd62637
CHANGELOG
ciampo Dec 15, 2021
ea21dff
Update docs and comments
ciampo Dec 17, 2021
7c2219f
Update unit tests
ciampo Dec 17, 2021
86092de
Update snapshots
ciampo Dec 17, 2021
b8abecd
Fix README formatting
ciampo Dec 17, 2021
625a976
Improve README
ciampo Jan 21, 2022
e4cd63c
Memoize `navigatorContextValue`
ciampo Jan 21, 2022
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
Next Next commit
Navigator: add basic location history
  • Loading branch information
@ciampo
ciampo committed Jan 21, 2022
commit c49be7355e279674b41ff234861b96eeadb9a6e4
6 changes: 5 additions & 1 deletion packages/components/src/navigator/context.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,9 @@ import { createContext } from '@wordpress/element';
*/
import type { NavigatorContext as NavigatorContextType } from './types';

const initialContextValue: NavigatorContextType = [ {}, () => {} ];
const initialContextValue: NavigatorContextType = {
location: {},
push: () => {},
pop: () => {},
};
export const NavigatorContext = createContext( initialContextValue );
60 changes: 54 additions & 6 deletions packages/components/src/navigator/navigator-provider/component.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { css } from '@emotion/react';
/**
* WordPress dependencies
*/
import { useMemo, useState } from '@wordpress/element';
import { useMemo, useState, useCallback } from '@wordpress/element';

/**
* Internal dependencies
Expand All @@ -20,7 +20,11 @@ import {
import { useCx } from '../../utils/hooks/use-cx';
import { View } from '../../view';
import { NavigatorContext } from '../context';
import type { NavigatorProviderProps, NavigatorPath } from '../types';
import type {
NavigatorProviderProps,
NavigatorLocation,
NavigatorContext as NavigatorContextType,
} from '../types';

function NavigatorProvider(
props: WordPressComponentProps< NavigatorProviderProps, 'div' >,
Expand All @@ -33,9 +37,53 @@ function NavigatorProvider(
...otherProps
} = useContextSystem( props, 'NavigatorProvider' );

const [ path, setPath ] = useState< NavigatorPath >( {
path: initialPath,
} );
const [ locationHistory, setLocationHistory ] = useState<
NavigatorLocation[]
>( [
{
path: initialPath,
isBack: false,
isInitial: true,
},
] );

const push: NavigatorContextType[ 'push' ] = useCallback(
( path, options ) => {
setLocationHistory( [
Copy link
Contributor

@youknowriad youknowriad Dec 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should add a limit to this array (like 10 items or so)

Copy link
Contributor Author

@ciampo ciampo Jan 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would be the motivation for it?

Adding a limit would have a few consequences. Let's say the limit is 10 — it means that, when we navigate to the 11th location, we would either:

  1. prevent any further navigation (since the stack is full). This approach doesn't seem very viable in terms of UX, since it would suddenly prevent the user from navigating to another screen. OR
  2. drop the initial root location, and make the second location in the array the new "root". This would introduce further complexity in the code (shifting the array, re-setting the isInitial prop, ...) and it would also mean that the user wouldn't be able to navigate back all the way to the initial route.

I personally think that, rather than enforcing a hard limit on the array, we should tackle this potential issue by making sure that our UIs are not designed to require too many levels of "screen nesting" in the first place.

Copy link
Contributor Author

@ciampo ciampo Jan 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @youknowriad , do you have any additional thoughts on my previous answer?

Copy link
Contributor

@youknowriad youknowriad Jan 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I didn't reply here. I was just thinking about memory leaks but as you said, it might not grow much so fine by me.

Copy link
Contributor Author

@ciampo ciampo Jan 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine, thank you!

...locationHistory.slice( 0, -1 ),
{
...locationHistory[ locationHistory.length - 1 ],
isBack: false,
},
{
...options,
path,
isBack: false,
isInitial: false,
},
] );
},
[ locationHistory ]
);

const pop: NavigatorContextType[ 'pop' ] = useCallback( () => {
if ( locationHistory.length > 1 ) {
setLocationHistory( [
...locationHistory.slice( 0, -2 ),
// Force the `isBack` flag to `true` when navigating back.
{
...locationHistory[ locationHistory.length - 2 ],
isBack: true,
},
] );
}
}, [ locationHistory ] );

const navigatorContextValue: NavigatorContextType = {
location: locationHistory[ locationHistory.length - 1 ],
push,
pop,
};

const cx = useCx();
const classes = useMemo(
Expand All @@ -46,7 +94,7 @@ function NavigatorProvider(

return (
<View ref={ forwardedRef } className={ classes } { ...otherProps }>
<NavigatorContext.Provider value={ [ path, setPath ] }>
<NavigatorContext.Provider value={ navigatorContextValue }>
{ children }
</NavigatorContext.Provider>
</View>
Expand Down
10 changes: 4 additions & 6 deletions packages/components/src/navigator/navigator-screen/component.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ function NavigatorScreen( props: Props, forwardedRef: Ref< any > ) {
);

const prefersReducedMotion = useReducedMotion();
const [ currentPath ] = useContext( NavigatorContext );
const isMatch = currentPath.path === path;
const { location } = useContext( NavigatorContext );
const isMatch = location.path === path;
const ref = useFocusOnMount();

const cx = useCx();
Expand Down Expand Up @@ -95,17 +95,15 @@ function NavigatorScreen( props: Props, forwardedRef: Ref< any > ) {
const initial = {
opacity: 0,
x:
( isRTL() && currentPath.isBack ) ||
( ! isRTL() && ! currentPath.isBack )
( isRTL() && location.isBack ) || ( ! isRTL() && ! location.isBack )
? 50
: -50,
};
const exit = {
delay: animationExitDelay,
opacity: 0,
x:
( ! isRTL() && currentPath.isBack ) ||
( isRTL() && ! currentPath.isBack )
( ! isRTL() && location.isBack ) || ( isRTL() && ! location.isBack )
? 50
: -50,
transition: {
Expand Down
23 changes: 11 additions & 12 deletions packages/components/src/navigator/stories/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,22 @@ export default {
component: NavigatorProvider,
};

function NavigatorButton( { path, isBack = false, ...props } ) {
const navigator = useNavigator();
function NavigatorButton( { path, ...props } ) {
const { push } = useNavigator();
return (
<Button
variant="secondary"
onClick={ () => navigator.push( path, { isBack } ) }
onClick={ () => push( path ) }
{ ...props }
/>
);
}

function NavigatorBackButton( props ) {
const { pop } = useNavigator();
return <Button variant="secondary" onClick={ () => pop() } { ...props } />;
}

const MyNavigation = () => {
const cx = useCx();
return (
Expand Down Expand Up @@ -74,19 +79,15 @@ const MyNavigation = () => {
<Card>
<CardBody>
<p>This is the child screen.</p>
<NavigatorButton path="/" isBack>
Go back
</NavigatorButton>
<NavigatorBackButton>Go back</NavigatorBackButton>
</CardBody>
</Card>
</NavigatorScreen>

<NavigatorScreen path="/overflow-child">
<Card>
<CardBody>
<NavigatorButton path="/" isBack>
Go back
</NavigatorButton>
<NavigatorBackButton>Go back</NavigatorBackButton>
<div
className={ cx(
css( `
Expand Down Expand Up @@ -114,9 +115,7 @@ const MyNavigation = () => {
<NavigatorScreen path="/stickies">
<Card>
<Sticky as={ CardHeader } z="2">
<NavigatorButton path="/" isBack>
Go back
</NavigatorButton>
<NavigatorBackButton>Go back</NavigatorBackButton>
</Sticky>
<CardBody>
<Sticky top="69px" colors="papayawhip/peachpuff">
Expand Down
21 changes: 10 additions & 11 deletions packages/components/src/navigator/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,22 @@
*/
import type { ReactNode } from 'react';

type NavigatorPathOptions = {
isBack?: boolean;
};
type NavigateOptions = {};

export type NavigatorPath = NavigatorPathOptions & {
export type NavigatorLocation = NavigateOptions & {
isInitial?: boolean;
isBack?: boolean;
path?: string;
};

export type NavigatorContext = [
NavigatorPath,
( path: NavigatorPath ) => void
];
export type NavigatorContext = {
location: NavigatorLocation;
push: ( path: string, options: NavigateOptions ) => void;
pop: () => void;
};

// Returned by the `useNavigator` hook
export type Navigator = {
push: ( path: string, options: NavigatorPathOptions ) => void;
};
export type Navigator = NavigatorContext;

export type NavigatorProviderProps = {
/**
Expand Down
8 changes: 4 additions & 4 deletions packages/components/src/navigator/use-navigator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ import type { Navigator } from './types';
* Retrieves a `navigator` instance.
*/
function useNavigator(): Navigator {
const [ , setPath ] = useContext( NavigatorContext );
const { location, push, pop } = useContext( NavigatorContext );

return {
push( path, options ) {
setPath( { path, ...options } );
},
location,
push,
pop,
};
}

Expand Down