Skip to content

Update Navigator styling to facilitate sticky positioning #35518

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
stokesman merged 6 commits into from
Oct 12, 2021
Merged

Update Navigator styling to facilitate sticky positioning #35518

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1beee74
Add style to facilitate sticky positioning within `Navigator`
stokesman Oct 6, 2021
6fad862
Add screen with sticky position to Navigator story
stokesman Oct 10, 2021
283db02
Use classnames hook instead of inline styles
stokesman Oct 12, 2021
0db480d
Pare down filler text component
stokesman Oct 12, 2021
a29b9ce
Fix casing of style rules
stokesman Oct 12, 2021
d8b66a4
Differentiate buttons
stokesman Oct 12, 2021
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
Prev Previous commit
Next Next commit
Add screen with sticky position to Navigator story
  • Loading branch information
@stokesman
stokesman committed Oct 11, 2021
commit 6fad8620c3c73484730367b6ec31f9716ccb6148
204 changes: 143 additions & 61 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 @@ -2,7 +2,7 @@
* Internal dependencies
*/
import Button from '../../button';
import { Card, CardBody, CardHeader } from '../../card';
import { Card, CardBody, CardFooter, CardHeader } from '../../card';
import { HStack } from '../../h-stack';
import { Flyout } from '../../flyout';
import { NavigatorProvider, NavigatorScreen, useNavigator } from '../';
Expand All @@ -22,74 +22,156 @@ function NavigatorButton( { path, isBack = false, ...props } ) {
);
}

const MyNavigation = () => {
return (
<NavigatorProvider initialPath="/">
<NavigatorScreen path="/">
<Card>
<CardBody>
<p>This is the home screen.</p>

<HStack justify="flex-start" wrap>
<NavigatorButton isPrimary path="/child">
Navigate to child screen.
</NavigatorButton>

<NavigatorButton path="/overflow-child">
Navigate to screen with horizontal overflow.
</NavigatorButton>
const MyNavigation = () => (
<NavigatorProvider
initialPath="/"
style={ { height: '100vh', maxHeight: '450px' } }
>
<NavigatorScreen path="/">
<Card>
<CardBody>
<p>This is the home screen.</p>

<Flyout
trigger={ <Button>Open test dialog</Button> }
placement="bottom-start"
>
<CardHeader>Go</CardHeader>
<CardBody>Stuff</CardBody>
</Flyout>
</HStack>
</CardBody>
</Card>
</NavigatorScreen>
<HStack justify="flex-start" wrap>
<NavigatorButton variant="primary" path="/child">
Navigate to child screen.
</NavigatorButton>

<NavigatorScreen path="/child">
<Card>
<CardBody>
<p>This is the child screen.</p>
<NavigatorButton isPrimary path="/" isBack>
Go back
<NavigatorButton path="/overflow-child">
Copy link
Contributor

@ciampo ciampo Oct 11, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Unrelated to the changes in this PR, but — should mark this button (and all NavigatorButton) as variant="primary" ?

Copy link
Contributor Author

@stokesman stokesman Oct 12, 2021

Choose a reason for hiding this comment

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

Ah, I was making some tangential changes and it's good to review them. So besides updating to the variant prop, I was trying to follow the button placement guidelines, specifically:

Don’t: Don’t place two primary buttons next to one another — they compete for focus. Only use one primary button per view.

So for a screen that only has a back back button it's probably fine to make primary, though I felt it was probably okay as default since an actual screen probably wouldn't consider the back button primary.

I'm open to suggestions. An idea I've considered is having the root screen use a ItemGroup instead of individual buttons.

Copy link
Contributor

@ciampo ciampo Oct 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

That's a good point. I guess I only wanted to differentiate the "links" from the button opening the modal — maybe we could then use secondary instead?

Something like this 
diff --git a/packages/components/src/navigator/stories/index.js b/packages/components/src/navigator/stories/index.js
index fe40eb67a1..97e527216e 100644
--- a/packages/components/src/navigator/stories/index.js
+++ b/packages/components/src/navigator/stories/index.js
@@ -22,6 +22,7 @@ function NavigatorButton( { path, isBack = false, ...props } ) {
 	const navigator = useNavigator();
 	return (
 		<Button
+			variant="secondary"
 			onClick={ () => navigator.push( path, { isBack } ) }
 			{ ...props }
 		/>
@@ -41,7 +42,7 @@ const MyNavigation = () => {
 						<p>This is the home screen.</p>
 
 						<HStack justify="flex-start" wrap>
-							<NavigatorButton variant="primary" path="/child">
+							<NavigatorButton path="/child">
 								Navigate to child screen.
 							</NavigatorButton>
 
@@ -54,7 +55,11 @@ const MyNavigation = () => {
 							</NavigatorButton>
 
 							<Flyout
-								trigger={ <Button>Open test dialog</Button> }
+								trigger={
+									<Button variant="primary">
+										Open test dialog
+									</Button>
+								}
 								placement="bottom-start"
 							>
 								<CardHeader>Go</CardHeader>

Copy link
Contributor Author

@stokesman stokesman Oct 12, 2021

Choose a reason for hiding this comment

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

Thanks! I like it and have applied it.

Navigate to screen with horizontal overflow.
</NavigatorButton>
</CardBody>
</Card>
</NavigatorScreen>
<NavigatorScreen path="/overflow-child">
<Card>
<CardBody>
<NavigatorButton isPrimary path="/" isBack>
Go back

<NavigatorButton path="/stickies">
Navigate to screen with sticky content.
</NavigatorButton>
<div

<Flyout
trigger={ <Button>Open test dialog</Button> }
placement="bottom-start"
>
<CardHeader>Go</CardHeader>
<CardBody>Stuff</CardBody>
</Flyout>
</HStack>
</CardBody>
</Card>
</NavigatorScreen>

<NavigatorScreen path="/child">
<Card>
<CardBody>
<p>This is the child screen.</p>
<NavigatorButton path="/" isBack>
Go back
</NavigatorButton>
</CardBody>
</Card>
</NavigatorScreen>

<NavigatorScreen path="/overflow-child">
<Card>
<CardBody>
<NavigatorButton path="/" isBack>
Go back
</NavigatorButton>
<div
style={ {
display: 'inline-block',
background: 'papayawhip',
} }
>
<span
style={ {
display: 'inline-block',
background: 'papayawhip',
color: 'palevioletred',
whiteSpace: 'nowrap',
fontSize: '42vw',
} }
>
<span
style={ {
color: 'palevioletred',
whiteSpace: 'nowrap',
fontSize: '42vw',
} }
>
¯\_(ツ)_/¯
</span>
</div>
</CardBody>
</Card>
</NavigatorScreen>
</NavigatorProvider>
);
};
¯\_(ツ)_/¯
</span>
</div>
</CardBody>
</Card>
</NavigatorScreen>

<NavigatorScreen path="/stickies">
<Card>
<Sticky as={ CardHeader } z="2">
<NavigatorButton path="/" isBack>
Go back
</NavigatorButton>
</Sticky>
<CardBody>
<Sticky top="69px" colors="papayawhip/peachpuff">
<h2>A wild sticky element appears</h2>
</Sticky>
<MetaphorIpsum quantity={ 3 } />
</CardBody>
<CardBody>
<Sticky top="69px" colors="azure/paleturquoise">
<h2>Another wild sticky element appears</h2>
</Sticky>
<MetaphorIpsum quantity={ 3 } />
</CardBody>
<Sticky as={ CardFooter } colors="mistyrose/pink">
<Button variant="primary">Primary noop</Button>
</Sticky>
</Card>
</NavigatorScreen>
</NavigatorProvider>
);

export const _default = () => {
return <MyNavigation />;
};

function Sticky( {
as: Tag = 'div',
bottom = 0,
colors = 'whitesmoke/lightgrey',
style,
top = 0,
z: zIndex = 1,
...props
} ) {
const [ bgColor, dotColor ] = colors.split( '/' );
const strictStyle = {
top,
bottom,
zIndex,
display: 'flex',
position: 'sticky',
background: `radial-gradient(${ dotColor } 1px, ${ bgColor } 2px) 50%/1em 1em`,
textAlign: 'center',
};
const propsOut = { ...props, style: { ...style, ...strictStyle } };
return <Tag { ...propsOut } />;
}

function MetaphorIpsum( { quantity } ) {
const list = [
'A pan of the particle is assumed to be an untorn trout. We can assume that any instance of a lawyer can be construed as a peevish page. A dietician is a plushest pamphlet. The testy aunt comes from an ebon halibut.',
'A dish is the basement of a romania. If this was somewhat unclear, their picture was, in this moment, a rustred sink. A precipitation is a bridgeless need. Before begonias, aprils were only snowflakes.',
'Those toes are nothing more than violets. A blithesome map without ghanas is truly a equinox of sicklied squirrels. Those acknowledgments are nothing more than brians. Their salad was, in this moment, a steadfast step-grandmother.',
'However, a profit can hardly be considered a doughy subway without also being a maid. They were lost without the pictured melody that composed their cheese. The halibut of a betty becomes a model care. A match is a sunlike owner.',
'A loopy clarinet’s year comes with it the thought that the fenny step-son is an ophthalmologist. The literature would have us believe that a glabrate country is not but a rhythm. A beech is a rub from the right perspective. In ancient times few can name an unglossed walrus that isn’t an unspilt trial.',
'Authors often misinterpret the afterthought as a roseless mother-in-law, when in actuality it feels more like an uncapped thunderstorm. In recent years, some posit the tarry bottle to be less than acerb. They were lost without the unkissed timbale that composed their customer. A donna is a springtime breath.',
'It’s an undeniable fact, really; their museum was, in this moment, a snotty beef. The swordfishes could be said to resemble prowessed lasagnas. However, the rainier authority comes from a cureless soup. Unfortunately, that is wrong; on the contrary, the cover is a powder.',
];
// Shuffle the list
for ( let i = list.length - 1; i > 0; i-- ) {
// eslint-disable-next-line no-restricted-syntax
const randomIndex = Math.floor( Math.random() * ( i + 1 ) );
[ list[ i ], list[ randomIndex ] ] = [ list[ randomIndex ], list[ i ] ];
}
quantity = Math.min( list.length, quantity );
return (
<>
{ list.slice( 0, quantity ).map( ( text, key ) => (
<p style={ { maxWidth: '20em' } } key={ key }>
{ text }
</p>
) ) }
</>
);
}