Skip to content

Commit

Permalink
Try sticking the navigation screen footer to the bottom of the screen…
Browse files Browse the repository at this point in the history 
… at all times
  • Loading branch information
@andrewserong
andrewserong committed Jun 5, 2023
1 parent e196939 commit cee7081
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 62 deletions.
5 changes: 4 additions & 1 deletion packages/edit-site/src/components/sidebar-navigation-screen-page/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,10 @@ export default function SidebarNavigationScreenPage() {
<HStack
spacing={ 5 }
alignment="left"
className="edit-site-sidebar-navigation-screen-page__details"
className={ classnames(

This comment has been minimized.

Sign in to view

Copy link
@ramonjd

ramonjd Jun 5, 2023

Member

Is classnames needed here?

This comment has been minimized.

Sign in to view

Copy link
@andrewserong

andrewserong Jun 5, 2023

Author Contributor

I suppose not really, we could always concatenate the two classnames together in a single string. I usually lean toward using classnames when there are multiple classnames involved so that we don't have a really long string. Happy to remove the call to classnames, though!

This comment has been minimized.

Sign in to view

Copy link
@ramonjd

ramonjd Jun 6, 2023

Member

Ah, okay, thanks for the explainer.

So it's just for code aesthetics? It probably doesn't add any overhead, but for some reason my brain is wondering what part of classnames functionality is being relied upon here.

If it's just to make things easier to read, then why not fold?

					<HStack
						spacing={ 5 }
						alignment="left"
						className="edit-site-sidebar-navigation-screen-page__details
							edit-site-sidebar-navigation-screen-page__footer"
					>

Not a blocker by the way, just curious!

This comment has been minimized.

Sign in to view

Copy link
@andrewserong

andrewserong Jun 6, 2023

Author Contributor

I really think I was overthinking this one 😄, it looks just fine with the two on a single line to me now that I've looked at it in my code editor — I've updated in 9ca113c

Thanks for pointing it out!
'edit-site-sidebar-navigation-screen-page__details',
'edit-site-sidebar-navigation-screen-page__footer'
) }
>
<Text className="edit-site-sidebar-navigation-screen-page__details-label">
{ __( 'Last modified' ) }
Expand Down
6 changes: 6 additions & 0 deletions packages/edit-site/src/components/sidebar-navigation-screen-page/style.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@
}
}

.edit-site-sidebar-navigation-screen-page__footer {
padding-top: $grid-unit-10;
padding-bottom: $grid-unit-10;
padding-left: $grid-unit-20;
}

.edit-site-sidebar-navigation-screen-page__details {
.edit-site-sidebar-navigation-screen-page__details-label {
color: $gray-600;
Expand Down
128 changes: 67 additions & 61 deletions packages/edit-site/src/components/sidebar-navigation-screen/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,74 +42,80 @@ export default function SidebarNavigationScreen( {
const theme = getTheme( currentlyPreviewingTheme() );

return (
<VStack spacing={ 0 }>
<HStack
spacing={ 4 }
alignment="flex-start"
className="edit-site-sidebar-navigation-screen__title-icon"
<>
<VStack
className="edit-site-sidebar-navigation-screen__main"
spacing={ 0 }
justify="flex-start"
>
{ ! isRoot ? (
<NavigatorToParentButton
as={ SidebarButton }
icon={ isRTL() ? chevronRight : chevronLeft }
label={ __( 'Back' ) }
/>
) : (
<SidebarButton
icon={ isRTL() ? chevronRight : chevronLeft }
label={
! isPreviewingTheme()
? __( 'Go back to the Dashboard' )
: __( 'Go back to the theme showcase' )
}
href={
! isPreviewingTheme()
? dashboardLink || 'index.php'
: 'themes.php'
}
/>
) }
<Heading
className="edit-site-sidebar-navigation-screen__title"
color={ 'white' }
level={ 2 }
size={ 20 }
<HStack
spacing={ 4 }
alignment="flex-start"
className="edit-site-sidebar-navigation-screen__title-icon"
>
{ ! isPreviewingTheme()
? title
: sprintf(
'Previewing %1$s: %2$s',
theme?.name?.rendered,
title
) }
</Heading>
{ actions && (
<div className="edit-site-sidebar-navigation-screen__actions">
{ actions }
</div>
{ ! isRoot ? (
<NavigatorToParentButton
as={ SidebarButton }
icon={ isRTL() ? chevronRight : chevronLeft }
label={ __( 'Back' ) }
/>
) : (
<SidebarButton
icon={ isRTL() ? chevronRight : chevronLeft }
label={
! isPreviewingTheme()
? __( 'Go back to the Dashboard' )
: __( 'Go back to the theme showcase' )
}
href={
! isPreviewingTheme()
? dashboardLink || 'index.php'
: 'themes.php'
}
/>
) }
<Heading
className="edit-site-sidebar-navigation-screen__title"
color={ 'white' }
level={ 2 }
size={ 20 }
>
{ ! isPreviewingTheme()
? title
: sprintf(
'Previewing %1$s: %2$s',
theme?.name?.rendered,
title
) }
</Heading>
{ actions && (
<div className="edit-site-sidebar-navigation-screen__actions">
{ actions }
</div>
) }
</HStack>
{ meta && (
<>
<div className="edit-site-sidebar-navigation-screen__meta">
{ meta }
</div>
</>
) }
</HStack>
{ meta && (
<>
<div className="edit-site-sidebar-navigation-screen__meta">
{ meta }
</div>
</>
) }

<nav className="edit-site-sidebar-navigation-screen__content">
{ description && (
<p className="edit-site-sidebar-navigation-screen__description">
{ description }
</p>
) }
{ content }
</nav>
<nav className="edit-site-sidebar-navigation-screen__content">
{ description && (
<p className="edit-site-sidebar-navigation-screen__description">
{ description }
</p>
) }
{ content }
</nav>
</VStack>
{ footer && (
<footer className="edit-site-sidebar-navigation-screen__sticky-section">
<footer className="edit-site-sidebar-navigation-screen__footer">
{ footer }
</footer>
) }
</VStack>
</>
);
}
13 changes: 13 additions & 0 deletions packages/edit-site/src/components/sidebar-navigation-screen/style.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@
position: relative;
}

.edit-site-sidebar-navigation-screen__main {
// Ensure the sidebar is always at least as tall as the viewport.
// This allows the footer section to be sticky at the bottom of the viewport.
flex-grow: 1;

This comment has been minimized.

Sign in to view

Copy link
@ramonjd

ramonjd Jun 5, 2023

Member

Nice idea to relocate the footer! It's working great for me in Safari/Firefox/Chrome 👍
}

.edit-site-sidebar-navigation-screen__content {
margin: 0 0 $grid-unit-20 0;
color: $gray-400;
Expand Down Expand Up @@ -86,3 +92,10 @@
margin: $grid-unit-40 $grid-unit-20;
border-top: 1px solid $gray-800;
}

.edit-site-sidebar-navigation-screen__footer {
position: sticky;
bottom: 0;
background-color: $gray-900;
padding: $grid-unit-20 0;
}
3 changes: 3 additions & 0 deletions packages/edit-site/src/components/sidebar/style.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
.components-navigator-screen {
@include custom-scrollbars-on-hover(transparent, $gray-700);
scrollbar-gutter: stable;
display: flex;
flex-direction: column;
height: 100%;
}
}

Expand Down

0 comments on commit cee7081

Please sign in to comment.