-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Site Editor: Add navigation panel with placeholder content #25506
Changes from all commits
2252995
2fe5b93
ea17f5e
c96a2cd
129e9fa
894a848
0531cf9
cbd4461
32027e4
9bab4e9
58dd95b
dbd5507
925fb27
1b21475
1b5133a
2b665bd
20bce36
76d897b
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 |
---|---|---|
|
@@ -26,13 +26,15 @@ import TemplateSwitcher from '../template-switcher'; | |
import SaveButton from '../save-button'; | ||
import UndoButton from './undo-redo/undo'; | ||
import RedoButton from './undo-redo/redo'; | ||
import FullscreenModeClose from './fullscreen-mode-close'; | ||
import DocumentActions from './document-actions'; | ||
import NavigationToggle from './navigation-toggle'; | ||
|
||
export default function Header( { | ||
openEntitiesSavedStates, | ||
isInserterOpen, | ||
onToggleInserter, | ||
isNavigationOpen, | ||
onToggleNavigation, | ||
} ) { | ||
const { | ||
deviceType, | ||
|
@@ -89,7 +91,10 @@ export default function Header( { | |
<div className="edit-site-header"> | ||
<div className="edit-site-header_start"> | ||
<MainDashboardButton.Slot> | ||
<FullscreenModeClose /> | ||
<NavigationToggle | ||
isOpen={ isNavigationOpen } | ||
onClick={ onToggleNavigation } | ||
/> | ||
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've got a few doubts about this change. What it does now is to toggle the navigation sidebar, so I'd be more inclined in calling it I think we could also keep Either way, my opinion is that we should probably avoid removing existing components from an experimental PR whose purpose is to be a starting point, and not the finished product. 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 agree that that name is more appropriate. Also no objections to keeping the 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. My point is also that the navigation sidebar is still in flux, and I'm not super keen on removing an established component (albeit experimental). 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.
|
||
</MainDashboardButton.Slot> | ||
<div className="edit-site-header__toolbar"> | ||
<Button | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { useSelect } from '@wordpress/data'; | ||
import { Button, Icon } from '@wordpress/components'; | ||
import { __ } from '@wordpress/i18n'; | ||
import { wordpress } from '@wordpress/icons'; | ||
|
||
function NavigationToggle( { icon, isOpen, onClick } ) { | ||
const { | ||
isActive, | ||
isRequestingSiteIcon, | ||
siteIconUrl, | ||
siteTitle, | ||
} = useSelect( ( select ) => { | ||
const { isFeatureActive } = select( 'core/edit-site' ); | ||
const { getEntityRecord } = select( 'core' ); | ||
const { isResolving } = select( 'core/data' ); | ||
const siteData = | ||
getEntityRecord( 'root', '__unstableBase', undefined ) || {}; | ||
|
||
return { | ||
isActive: isFeatureActive( 'fullscreenMode' ), | ||
isRequestingSiteIcon: isResolving( 'core', 'getEntityRecord', [ | ||
'root', | ||
'__unstableBase', | ||
undefined, | ||
] ), | ||
siteIconUrl: siteData.site_icon_url, | ||
siteTitle: siteData.name, | ||
}; | ||
}, [] ); | ||
|
||
if ( ! isActive ) { | ||
return null; | ||
} | ||
|
||
let buttonIcon = <Icon size="32px" icon={ wordpress } />; | ||
|
||
if ( siteIconUrl ) { | ||
buttonIcon = ( | ||
<img | ||
alt={ __( 'Site Icon' ) } | ||
className="edit-site-navigation-toggle__site-icon" | ||
src={ siteIconUrl } | ||
/> | ||
); | ||
} else if ( isRequestingSiteIcon ) { | ||
buttonIcon = null; | ||
} else if ( icon ) { | ||
buttonIcon = <Icon size="32px" icon={ icon } />; | ||
} | ||
|
||
return ( | ||
<div | ||
className={ | ||
'edit-site-navigation-toggle' + ( isOpen ? ' is-open' : '' ) | ||
} | ||
> | ||
<Button | ||
className="edit-site-navigation-toggle__button has-icon" | ||
label={ __( 'Toggle navigation' ) } | ||
onClick={ onClick } | ||
showTooltip | ||
> | ||
{ buttonIcon } | ||
</Button> | ||
|
||
{ isOpen && ( | ||
<div className="edit-site-navigation-toggle__site-title"> | ||
{ siteTitle } | ||
</div> | ||
) } | ||
</div> | ||
); | ||
} | ||
|
||
export default NavigationToggle; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
.edit-site-navigation-toggle { | ||
display: none; | ||
|
||
@include break-medium() { | ||
display: flex; | ||
align-items: center; | ||
background-color: #1e1e1e; | ||
height: 61px; | ||
border-radius: 0; | ||
} | ||
} | ||
|
||
.edit-site-navigation-toggle.is-open { | ||
width: 300px; | ||
} | ||
|
||
.edit-site-navigation-toggle__button { | ||
color: #fff; | ||
margin-left: 14px; | ||
margin-right: 14px; | ||
|
||
&:hover { | ||
color: #ddd; | ||
} | ||
} | ||
|
||
.edit-site-navigation-toggle.components-button.has-icon { | ||
justify-content: flex-start; | ||
padding: 0; | ||
height: 32px; | ||
width: 32px; | ||
min-width: 32px; | ||
} | ||
|
||
.edit-site-navigation-toggle__site-title { | ||
font-style: normal; | ||
font-weight: 600; | ||
font-size: 13px; | ||
line-height: 16px; | ||
color: #ddd; | ||
margin-right: 14px; | ||
|
||
display: -webkit-box; | ||
-webkit-line-clamp: 2; | ||
-webkit-box-orient: vertical; | ||
overflow: hidden; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import { render } from '@testing-library/react'; | ||
|
||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { useSelect } from '@wordpress/data'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import NavigationToggle from '..'; | ||
|
||
jest.mock( '@wordpress/data/src/components/use-select', () => { | ||
// This allows us to tweak the returned value on each test | ||
const mock = jest.fn(); | ||
return mock; | ||
} ); | ||
|
||
jest.mock( '@wordpress/core-data' ); | ||
|
||
describe( 'NavigationToggle', () => { | ||
describe( 'when in full screen mode', () => { | ||
it( 'should display a user uploaded site icon if it exists', () => { | ||
useSelect.mockImplementation( ( cb ) => { | ||
return cb( () => ( { | ||
isResolving: () => false, | ||
isFeatureActive: () => true, | ||
getEntityRecord: () => ( { | ||
site_icon_url: 'https://fakeUrl.com', | ||
} ), | ||
} ) ); | ||
} ); | ||
|
||
const { container } = render( <NavigationToggle /> ); | ||
const siteIcon = container.querySelector( | ||
'.edit-site-navigation-toggle__site-icon' | ||
); | ||
|
||
expect( siteIcon ).toBeTruthy(); | ||
} ); | ||
|
||
it( 'should display a default site icon if no user uploaded site icon exists', () => { | ||
useSelect.mockImplementation( ( cb ) => { | ||
return cb( () => ( { | ||
isResolving: () => false, | ||
isFeatureActive: () => true, | ||
getEntityRecord: () => ( { | ||
site_icon_url: '', | ||
} ), | ||
} ) ); | ||
} ); | ||
|
||
const { container } = render( <NavigationToggle /> ); | ||
const siteIcon = container.querySelector( | ||
'.edit-site-navigation-toggle__site-icon' | ||
); | ||
const defaultIcon = container.querySelector( 'svg' ); | ||
|
||
expect( siteIcon ).toBeFalsy(); | ||
expect( defaultIcon ).toBeTruthy(); | ||
} ); | ||
} ); | ||
} ); |
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.
This button is exported from the package (experimental for now) and provides a way for 3rd plugins to customize some of its parts while retaining functionality (see docs here). Given that it will no longer represent a close action, I think that we should rename the existing
FullscreenModeClose
toNavigationButton
orDashboardButton
and place our code changes there.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.
We should also figure out how to handle the FSE Navigation when not in fullscreen mode, where we wouldn't have the WP/Site Icon back button anymore.
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.
Good point @Copons! I don't consider it a blocker for this PR though, as it's something that we can figure out later.