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

Edit Site: New page based navigation. #22368

Merged
merged 12 commits into from
May 19, 2020
Merged

Conversation

epiqueras
Copy link
Contributor

Closes #19257

Description

This PR adds a new way of navigating in the site editor.

It adds a new page switcher dropdown that lets you select from your list of pages and category pages. The list can be expanded to include other pages in the future. The idea is to have one option for every unique URL path that the site exposes, except for post permalinks. A next step here would be to add a Post section that lets you load the single post page with a specific post context using a nested dropdown with the link UI.

Selecting a page switches the current template to the one resolved for that page and sets the correct root block context, so that post and query blocks work as expected. This flow is shown in the GIF below, where switching to the contact page makes the Post Title block render its title. Category pages also work like this, but the query blocks still need some work to read root context so that part is not testable just yet.

The next step would be to modify the template switcher so that it not only switches the current template but also provides a way to assign it to the current page.

This PR also:

  • Fixes some subtle bugs that were introduced by the new API refactors and optimizes template auto-draft creation so that they are only recreated when the underlying block template files have changed.
  • Adds a fallback index template to the demo templates to facilitate testing.

How has this been tested?

The flow that is shown in the GIF where switching pages in the site editor will update the current template and root block context was tested with a combination of different templates and pages to verify that the correct template is resolved for each page and that the root block context is set correctly.

Screenshots

gif

Types of Changes

New Feature: The site editor now has a page switcher for navigating between pages instead of templates.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@epiqueras epiqueras added [Type] Performance Related to performance efforts [Feature] Full Site Editing [Type] Feature New feature to highlight in changelogs. labels May 14, 2020
@epiqueras epiqueras requested a review from ockham May 14, 2020 21:23
@epiqueras epiqueras self-assigned this May 14, 2020
@github-actions
Copy link

github-actions bot commented May 14, 2020

Size Change: +655 B (0%)

Total Size: 833 kB

Filename Size Change
build/block-editor/index.js 104 kB -1 B
build/block-library/index.js 118 kB +142 B (0%)
build/blocks/index.js 48.1 kB +2 B (0%)
build/core-data/index.js 11.4 kB -1 B
build/edit-post/index.js 28.1 kB -1 B
build/edit-site/index.js 12.5 kB +510 B (4%)
build/edit-widgets/index.js 7.87 kB +2 B (0%)
build/keyboard-shortcuts/index.js 2.51 kB +1 B
build/rich-text/index.js 14.8 kB +1 B
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.02 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 3.39 kB 0 B
build/autop/index.js 2.83 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.59 kB 0 B
build/block-directory/style-rtl.css 764 B 0 B
build/block-directory/style.css 764 B 0 B
build/block-editor/style-rtl.css 10.8 kB 0 B
build/block-editor/style.css 10.8 kB 0 B
build/block-library/editor-rtl.css 7.19 kB 0 B
build/block-library/editor.css 7.19 kB 0 B
build/block-library/style-rtl.css 7.48 kB 0 B
build/block-library/style.css 7.48 kB 0 B
build/block-library/theme-rtl.css 683 B 0 B
build/block-library/theme.css 685 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/components/index.js 182 kB 0 B
build/components/style-rtl.css 17.1 kB 0 B
build/components/style.css 17 kB 0 B
build/compose/index.js 6.67 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.42 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.1 kB 0 B
build/edit-navigation/index.js 5.6 kB 0 B
build/edit-navigation/style-rtl.css 618 B 0 B
build/edit-navigation/style.css 617 B 0 B
build/edit-post/style-rtl.css 12.2 kB 0 B
build/edit-post/style.css 12.2 kB 0 B
build/edit-site/style-rtl.css 5.22 kB 0 B
build/edit-site/style.css 5.22 kB 0 B
build/edit-widgets/style-rtl.css 4.69 kB 0 B
build/edit-widgets/style.css 4.69 kB 0 B
build/editor/editor-styles-rtl.css 425 B 0 B
build/editor/editor-styles.css 428 B 0 B
build/editor/index.js 44.3 kB 0 B
build/editor/style-rtl.css 5.07 kB 0 B
build/editor/style.css 5.08 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.63 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 711 B 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.13 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 5.29 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/server-side-render/index.js 2.68 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.02 kB 0 B
build/viewport/index.js 1.84 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

Copy link
Member

@vindl vindl left a comment

Choose a reason for hiding this comment

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

Glad to see this coming together! 🚀

Here are some of observations from my recent testing:

The page dropdown button is not visible on initial load.

Screenshot 2020-05-15 at 20 01 59

Although it does get populated with a page title after clicking on that area and selecting one of the pages. The position of dropdown shifts slightly while toggling pages, depending on the size of the title.

There are also some test failures that seem to be related to this PR.

@epiqueras
Copy link
Contributor Author

The page dropdown button is not visible on initial load.

This is because you don't have a static front page, and the PR didn't include the "All Posts" page yet. I just added it.

The position of dropdown shifts slightly while toggling pages, depending on the size of the title.

We need to combine the two dropdowns in another PR.

There are also some test failures that seem to be related to this PR.

Fixtures, updated.

I also added support for category pages now:

gif

@epiqueras epiqueras merged commit 02fe765 into master May 19, 2020
@epiqueras epiqueras deleted the add/new-site-editor-navigation branch May 19, 2020 00:07
@github-actions github-actions bot added this to the Gutenberg 8.2 milestone May 19, 2020
@@ -56,6 +67,33 @@ export default function Header( { openEntitiesSavedStates } ) {
} ) ),
[]
);
const setActivePage = useCallback( async ( newPage ) => {
try {
const { success, data } = await fetch(
Copy link
Contributor

Choose a reason for hiding this comment

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

Just stumbled upon this and it seems a bit hacky. What does this fetch do? Why not core-data? Why not apiFetch?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, it seems to me that all the logic here is better on an async store action. (generator with controls) as otherwise there's a lot of potential for breakage as written (set state on unmounted components).

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess the issue is that "settings" is not yet on a store, for me, this is becoming more and more a priority as we add components and share state.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not precisely an API endpoint. It's the only straightforward way of triggering Core's template resolution for a URL because calling parse_request in WP twice is not trivial.

Yes, I agree that at this point, it makes sense to move settings into a store and have this be an action.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm also wondering if we're making things hard for us by trying to act like a full SPA here where we could instead trigger a reload and append something to the URL? (The dirty detection check could help us to avoid leaving the page with unsaved changes).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could add deep linking without refreshes too.

I do think that linking to a specific page in the site editor from different places would be nice.

Copy link
Member

Choose a reason for hiding this comment

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

I'm also wondering if we're making things hard for us by trying to act like a full SPA here where we could instead trigger a reload and append something to the URL?

IMO, it would be nice to avoid hard refreshes. One of the biggest user complaints on WordPress.com is how long the block editor takes to load, which is a combination of loading an iframe and loading the block editor. Destroying the existing JS context and rebuilding it seems to be really inefficient (IMO) when the code can already handle switching pages client side! Since switching pages and templates seems to be a common task, it’d be nice to keep that interaction quick :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Feature New feature to highlight in changelogs. [Type] Performance Related to performance efforts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Edit Site: Loading a specific context on a dynamic template
4 participants