Skip to content
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

Rewrite URL (Permalink) panel as a popover #42033

Merged
merged 8 commits into from
Jul 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,17 @@ import {
activatePlugin,
createNewPost,
deactivatePlugin,
findSidebarPanelWithTitle,
publishPost,
} from '@wordpress/e2e-test-utils';

const permalinkPanelXPath = `//div[contains(@class, "edit-post-sidebar")]//button[contains(@class, "components-panel__body-toggle") and contains(text(),"Permalink")]`;
// TODO: Use a more accessible selector.
Copy link
Member Author

Choose a reason for hiding this comment

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

I'll fix this in a follow up PR. It points to how we need to set proper aria-labels on these buttons. Right now screen readers read them as Public, June 27, example.com/page, Default template which I can't imagine is very useful. It should be something like Select visibility: Public, Change publish date: June 27, Change URL: example.com/page, Change template: Default template.

Why a separate PR and not now? Because the issue already exists in trunk and I want proper a11y / copy feedback.

Copy link
Member

Choose a reason for hiding this comment

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

We can also migrate the e2e tests to Playwright while we are at it 😄 The locator works great with labels.

const urlRowSelector = '.edit-post-post-url';

// This tests are not together with the remaining sidebar tests,
// because we need to publish/save a post, to correctly test the permalink panel.
// because we need to publish/save a post, to correctly test the permalink row.
// The sidebar test suit enforces that focus is never lost, but during save operations
// the focus is lost and a new element is focused once the save is completed.
describe( 'Sidebar Permalink Panel', () => {
describe( 'Sidebar Permalink', () => {
beforeAll( async () => {
await activatePlugin( 'gutenberg-test-custom-post-types' );
} );
Expand All @@ -24,39 +24,30 @@ describe( 'Sidebar Permalink Panel', () => {
await deactivatePlugin( 'gutenberg-test-custom-post-types' );
} );

it( 'should allow permalink sidebar panel to be removed', async () => {
await createNewPost();
await page.evaluate( () => {
const { removeEditorPanel } = wp.data.dispatch( 'core/edit-post' );
removeEditorPanel( 'post-link' );
} );
expect( await page.$x( permalinkPanelXPath ) ).toEqual( [] );
} );

it( 'should not render link panel when post is publicly queryable but not public', async () => {
it( 'should not render URL when post is publicly queryable but not public', async () => {
await createNewPost( { postType: 'public_q_not_public' } );
await page.keyboard.type( 'aaaaa' );
await publishPost();
// Start editing again.
await page.type( '.editor-post-title__input', ' (Updated)' );
expect( await page.$x( permalinkPanelXPath ) ).toEqual( [] );
expect( await page.$( urlRowSelector ) ).toBeNull();
} );

it( 'should not render link panel when post is public but not publicly queryable', async () => {
it( 'should not render URL when post is public but not publicly queryable', async () => {
await createNewPost( { postType: 'not_public_q_public' } );
await page.keyboard.type( 'aaaaa' );
await publishPost();
// Start editing again.
await page.type( '.editor-post-title__input', ' (Updated)' );
expect( await page.$x( permalinkPanelXPath ) ).toEqual( [] );
expect( await page.$( urlRowSelector ) ).toBeNull();
} );

it( 'should render link panel when post is public and publicly queryable', async () => {
it( 'should render URL when post is public and publicly queryable', async () => {
await createNewPost( { postType: 'public_q_public' } );
await page.keyboard.type( 'aaaaa' );
await publishPost();
// Start editing again.
await page.type( '.editor-post-title__input', ' (Updated)' );
expect( await findSidebarPanelWithTitle( 'Permalink' ) ).toBeDefined();
expect( await page.$( urlRowSelector ) ).not.toBeNull();
} );
} );
22 changes: 5 additions & 17 deletions packages/edit-post/src/components/preferences-modal/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import {
PostTypeSupportCheck,
store as editorStore,
} from '@wordpress/editor';
import { store as coreStore } from '@wordpress/core-data';
import {
PreferencesModal,
PreferencesModalTabs,
Expand All @@ -45,15 +44,10 @@ const MODAL_NAME = 'edit-post/preferences';
export default function EditPostPreferencesModal() {
const isLargeViewport = useViewportMatch( 'medium' );
const { closeModal } = useDispatch( editPostStore );
const { isModalActive, isViewable } = useSelect( ( select ) => {
const { getEditedPostAttribute } = select( editorStore );
const { getPostType } = select( coreStore );
const postType = getPostType( getEditedPostAttribute( 'type' ) );
return {
isModalActive: select( editPostStore ).isModalActive( MODAL_NAME ),
isViewable: get( postType, [ 'viewable' ], false ),
};
}, [] );
const isModalActive = useSelect(
( select ) => select( editPostStore ).isModalActive( MODAL_NAME ),
[]
);
const showBlockBreadcrumbsOption = useSelect(
( select ) => {
const { getEditorSettings } = select( editorStore );
Expand Down Expand Up @@ -200,12 +194,6 @@ export default function EditPostPreferencesModal() {
) }
>
<EnablePluginDocumentSettingPanelOption.Slot />
{ isViewable && (
<EnablePanelOption
label={ __( 'Permalink' ) }
panelName="post-link"
/>
) }
<PostTaxonomies
taxonomyWrapper={ ( content, taxonomy ) => (
<EnablePanelOption
Expand Down Expand Up @@ -254,7 +242,7 @@ export default function EditPostPreferencesModal() {
),
},
],
[ isViewable, isLargeViewport, showBlockBreadcrumbsOption ]
[ isLargeViewport, showBlockBreadcrumbsOption ]
);

if ( ! isModalActive ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,21 @@ jest.mock( '@wordpress/compose/src/hooks/use-viewport-match', () => jest.fn() );
describe( 'EditPostPreferencesModal', () => {
describe( 'should match snapshot when the modal is active', () => {
it( 'large viewports', () => {
useSelect.mockImplementation( () => ( { isModalActive: true } ) );
useSelect.mockImplementation( () => true );
useViewportMatch.mockImplementation( () => true );
const wrapper = shallow( <EditPostPreferencesModal /> );
expect( wrapper ).toMatchSnapshot();
} );
it( 'small viewports', () => {
useSelect.mockImplementation( () => ( { isModalActive: true } ) );
useSelect.mockImplementation( () => true );
useViewportMatch.mockImplementation( () => false );
const wrapper = shallow( <EditPostPreferencesModal /> );
expect( wrapper ).toMatchSnapshot();
} );
} );

it( 'should not render when the modal is not active', () => {
useSelect.mockImplementation( () => ( { isModalActive: false } ) );
useSelect.mockImplementation( () => false );
Copy link
Member

Choose a reason for hiding this comment

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

Select now just returns a boolean used for isModalActive.

const wrapper = shallow( <EditPostPreferencesModal /> );
expect( wrapper.isEmptyRender() ).toBe( true );
} );
Expand Down
179 changes: 0 additions & 179 deletions packages/edit-post/src/components/sidebar/post-link/index.js

This file was deleted.

20 changes: 0 additions & 20 deletions packages/edit-post/src/components/sidebar/post-link/style.scss

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,11 @@
width: 100%;
position: relative;
justify-content: flex-start;
align-items: flex-start;

span {
display: block;
width: 45%;
flex-shrink: 0;
padding: 6px 0;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import PostPendingStatus from '../post-pending-status';
import PluginPostStatusInfo from '../plugin-post-status-info';
import { store as editPostStore } from '../../../store';
import PostTemplate from '../post-template';
import PostURL from '../post-url';

/**
* Module Constants
Expand All @@ -39,6 +40,7 @@ function PostStatus( { isOpened, onTogglePanel } ) {
<>
<PostVisibility />
<PostSchedule />
<PostURL />
<PostTemplate />
<PostFormat />
<PostSticky />
Expand Down
Loading