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

Let Gutenberg be default editor for posts with blocks; add links to classic editor #1797

Merged
merged 11 commits into from Sep 8, 2017

Conversation

Projects
None yet
6 participants
@westonruter
Member

westonruter commented Jul 7, 2017

Fixes #1779, #1627.

When a post contains blocks, the main edit link becomes the link to Gutenberg whereas the row action link then goes to the Classic Editor:

image

Any edit post link for a post with blocks becomes a link to the Gutenberg editor, including links on the frontend:

image

If a post does not contain blocks, then the main link remains pointing to the classic editor and the Gutenberg link is in the row actions.

Likewise, if a post does have blocks then when the post is viewed on the frontend, the “Edit Post” link will link to Gutenberg and it will have a “Edit in Gutenberg” title. The classic editor will then be available via a submenu item:

image

Likewise, when the post does not contain blocks, then the original Edit Post link is in place with a link to Gutenberg being in the submenu as the alternate edit mode:

image

Lastly, if the post type is not registered with show_in_rest then the Gutenberg editor is not shown, since the REST API is a dependency for Gutenberg.

@westonruter

This comment has been minimized.

Show comment
Hide comment
@westonruter

westonruter Jul 7, 2017

Member

I'll add unit tests for the new PHP functions once a 👍 on the functionality is given.

Member

westonruter commented Jul 7, 2017

I'll add unit tests for the new PHP functions once a 👍 on the functionality is given.

@afercia

This comment has been minimized.

Show comment
Hide comment
@afercia

afercia Jul 9, 2017

Contributor

While there's the need to keep the links text as short as possible, given the limited space, I'd consider to try to improve them a bit.

  • all the row actions use verbs: Edit, Quick Edit, View, Preview, etc.: "Classic Editor" and "Gutenberg" instead don't immediately communicate the available action
  • the action of the "Edit" link in the list table and the "Edit Post" in the toolbar can be inferred only looking at the link close to them, not sure if this will be immediately clear for users

screen shot 2017-07-09 at 13 52 15
screen shot 2017-07-09 at 13 54 44
screen shot 2017-07-09 at 13 52 40
screen shot 2017-07-09 at 13 52 49

screen shot 2017-07-09 at 13 50 52
screen shot 2017-07-09 at 13 51 13

Since the aria-label text is not visible, it could be expanded always specifying "in the classic editor" / "in the Gutenberg editor"

Contributor

afercia commented Jul 9, 2017

While there's the need to keep the links text as short as possible, given the limited space, I'd consider to try to improve them a bit.

  • all the row actions use verbs: Edit, Quick Edit, View, Preview, etc.: "Classic Editor" and "Gutenberg" instead don't immediately communicate the available action
  • the action of the "Edit" link in the list table and the "Edit Post" in the toolbar can be inferred only looking at the link close to them, not sure if this will be immediately clear for users

screen shot 2017-07-09 at 13 52 15
screen shot 2017-07-09 at 13 54 44
screen shot 2017-07-09 at 13 52 40
screen shot 2017-07-09 at 13 52 49

screen shot 2017-07-09 at 13 50 52
screen shot 2017-07-09 at 13 51 13

Since the aria-label text is not visible, it could be expanded always specifying "in the classic editor" / "in the Gutenberg editor"

@westonruter

This comment has been minimized.

Show comment
Hide comment
@westonruter

westonruter Jul 10, 2017

Member

the action of the "Edit" link in the list table and the "Edit Post" in the toolbar can be inferred only looking at the link close to them, not sure if this will be immediately clear for users

@afercia I'm not entirely clear if your feedback here is about the design of the WordPress post list table in general, or if it is specifically regarding changes in this PR.

Member

westonruter commented Jul 10, 2017

the action of the "Edit" link in the list table and the "Edit Post" in the toolbar can be inferred only looking at the link close to them, not sure if this will be immediately clear for users

@afercia I'm not entirely clear if your feedback here is about the design of the WordPress post list table in general, or if it is specifically regarding changes in this PR.

@afercia

This comment has been minimized.

Show comment
Hide comment
@afercia

afercia Jul 10, 2017

Contributor

@westonruter I meant the current list table design in WordPress is clear enough because there's just one "Edit" and just one editor. Now that this PR introduces 2 links to 2 different editors, it's not always immediately clear where the "Edit" link points.

Contributor

afercia commented Jul 10, 2017

@westonruter I meant the current list table design in WordPress is clear enough because there's just one "Edit" and just one editor. Now that this PR introduces 2 links to 2 different editors, it's not always immediately clear where the "Edit" link points.

@westonruter

This comment has been minimized.

Show comment
Hide comment
@westonruter

westonruter Jul 10, 2017

Member

It links to the default editor. If you think it should be verbosely “Edit in Gutenberg”, I wouldn't be opposed, but again there is limited space.

Member

westonruter commented Jul 10, 2017

It links to the default editor. If you think it should be verbosely “Edit in Gutenberg”, I wouldn't be opposed, but again there is limited space.

@afercia

This comment has been minimized.

Show comment
Hide comment
@afercia

afercia Jul 10, 2017

Contributor

Yep that's what I meant 🙂 I see the limited space issue. Shortening the link text is not always so clear though.

Contributor

afercia commented Jul 10, 2017

Yep that's what I meant 🙂 I see the limited space issue. Shortening the link text is not always so clear though.

@westonruter

This comment has been minimized.

Show comment
Hide comment
@westonruter

westonruter Jul 10, 2017

Member

Please amend the PR with whatever you deem best.

Member

westonruter commented Jul 10, 2017

Please amend the PR with whatever you deem best.

Show outdated Hide outdated lib/register.php Outdated
@westonruter

This comment has been minimized.

Show comment
Hide comment
@westonruter

westonruter Jul 25, 2017

Member

Rebased. 326645d rewritten to ef5a661.

Member

westonruter commented Jul 25, 2017

Rebased. 326645d rewritten to ef5a661.

@karmatosed

This comment has been minimized.

Show comment
Hide comment
@karmatosed

karmatosed Aug 30, 2017

Member

+1 to getting this merged from me. Can we get a refresh to pass all checks? Be great to get this in for 1.1.

Member

karmatosed commented Aug 30, 2017

+1 to getting this merged from me. Can we get a refresh to pass all checks? Be great to get this in for 1.1.

@afercia

This comment has been minimized.

Show comment
Hide comment
@afercia

afercia Aug 30, 2017

Contributor

@karmatosed I kindly disagree. There's still a design issue to solve here: the link destination is not always clear.

Contributor

afercia commented Aug 30, 2017

@karmatosed I kindly disagree. There's still a design issue to solve here: the link destination is not always clear.

@karmatosed

This comment has been minimized.

Show comment
Hide comment
@karmatosed

karmatosed Aug 30, 2017

Member

@afercia to me, we get the PR in a state we can then see (it's not yet) and then either merge and iterate or iterate. What are you thinking is missing and would advise?

Member

karmatosed commented Aug 30, 2017

@afercia to me, we get the PR in a state we can then see (it's not yet) and then either merge and iterate or iterate. What are you thinking is missing and would advise?

@afercia

This comment has been minimized.

Show comment
Hide comment
@afercia

afercia Aug 31, 2017

Contributor

@karmatosed for example: would you be able to understand what the two "Edit" links in the screenshot below do at first sight?

screen shot 2017-08-31 at 09 03 49

To me, this seems confusing because I can understand the difference between the two "Edit" only by looking at the other link close to them. Not to mention, the link on the post title doesn't say anything either.

I see potential confusion also in the toolbar: often times, users just click, they don't read (muscle memory also). They will see "Edit bla bla" and click and sometimes land in the classic editor, sometimes on Gutenberg.

Contributor

afercia commented Aug 31, 2017

@karmatosed for example: would you be able to understand what the two "Edit" links in the screenshot below do at first sight?

screen shot 2017-08-31 at 09 03 49

To me, this seems confusing because I can understand the difference between the two "Edit" only by looking at the other link close to them. Not to mention, the link on the post title doesn't say anything either.

I see potential confusion also in the toolbar: often times, users just click, they don't read (muscle memory also). They will see "Edit bla bla" and click and sometimes land in the classic editor, sometimes on Gutenberg.

@aduth

This comment has been minimized.

Show comment
Hide comment
@aduth

aduth Sep 1, 2017

Member

To me, it seems like it might be good to have edit actions as "Gutenberg Editor" (or "Edit in Gutenberg"), "Classic Editor" (or "Edit in Classic Editor") in all cases (never changing order or text), then set the behavior of the title link to the preferred default (Gutenberg if blocks exist, Classic otherwise).

Member

aduth commented Sep 1, 2017

To me, it seems like it might be good to have edit actions as "Gutenberg Editor" (or "Edit in Gutenberg"), "Classic Editor" (or "Edit in Classic Editor") in all cases (never changing order or text), then set the behavior of the title link to the preferred default (Gutenberg if blocks exist, Classic otherwise).

@aduth

This comment has been minimized.

Show comment
Hide comment
@aduth

aduth Sep 1, 2017

Member

I've rebased to resolve conflicts and made the following revisions:

Using the display_post_states filter, add a "Gutenberg" status in post table for posts containing blocks:

Gutenberg status

Always show "Classic Editor" and "Gutenberg" links in post actions, regardless of post contents:

Post actions

Always show full "Edit in ..." text in front-end admin bar:

Admin bar

Member

aduth commented Sep 1, 2017

I've rebased to resolve conflicts and made the following revisions:

Using the display_post_states filter, add a "Gutenberg" status in post table for posts containing blocks:

Gutenberg status

Always show "Classic Editor" and "Gutenberg" links in post actions, regardless of post contents:

Post actions

Always show full "Edit in ..." text in front-end admin bar:

Admin bar

Show outdated Hide outdated lib/register.php Outdated
Show outdated Hide outdated lib/register.php Outdated
@aduth

This comment has been minimized.

Show comment
Hide comment
@aduth

aduth Sep 6, 2017

Member

54df7bc should resolve the issue where Classic Editor would use an incorrect link for Gutenberg posts (from discussion thread at #1797 (comment)).

From my perspective, this seems ready to go.

Member

aduth commented Sep 6, 2017

54df7bc should resolve the issue where Classic Editor would use an incorrect link for Gutenberg posts (from discussion thread at #1797 (comment)).

From my perspective, this seems ready to go.

@aduth

aduth approved these changes Sep 6, 2017

@codecov

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov bot Sep 6, 2017

Codecov Report

Merging #1797 into master will increase coverage by 1%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #1797    +/-   ##
========================================
+ Coverage   31.38%   32.39%    +1%     
========================================
  Files         177      182     +5     
  Lines        5413     5933   +520     
  Branches      949     1087   +138     
========================================
+ Hits         1699     1922   +223     
- Misses       3139     3348   +209     
- Partials      575      663    +88
Impacted Files Coverage Δ
blocks/editable/index.js 10.02% <0%> (-0.5%) ⬇️
editor/sidebar/post-status/index.js 0% <0%> (ø) ⬆️
blocks/library/gallery/gallery-image.js 50% <0%> (ø) ⬆️
blocks/editable/constants.js 100% <0%> (ø)
editor/sidebar/post-pending-status/index.js 58.33% <0%> (ø)
components/autocomplete/index.js 89.04% <0%> (ø)
blocks/block-autocomplete/index.js 100% <0%> (ø)
blocks/library/gallery/block.js 11.76% <0%> (ø)
blocks/editable/format-toolbar/index.js 8% <0%> (+0.5%) ⬆️
components/drop-zone/provider.js 1.98% <0%> (+0.64%) ⬆️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8621e54...54df7bc. Read the comment docs.

codecov bot commented Sep 6, 2017

Codecov Report

Merging #1797 into master will increase coverage by 1%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #1797    +/-   ##
========================================
+ Coverage   31.38%   32.39%    +1%     
========================================
  Files         177      182     +5     
  Lines        5413     5933   +520     
  Branches      949     1087   +138     
========================================
+ Hits         1699     1922   +223     
- Misses       3139     3348   +209     
- Partials      575      663    +88
Impacted Files Coverage Δ
blocks/editable/index.js 10.02% <0%> (-0.5%) ⬇️
editor/sidebar/post-status/index.js 0% <0%> (ø) ⬆️
blocks/library/gallery/gallery-image.js 50% <0%> (ø) ⬆️
blocks/editable/constants.js 100% <0%> (ø)
editor/sidebar/post-pending-status/index.js 58.33% <0%> (ø)
components/autocomplete/index.js 89.04% <0%> (ø)
blocks/block-autocomplete/index.js 100% <0%> (ø)
blocks/library/gallery/block.js 11.76% <0%> (ø)
blocks/editable/format-toolbar/index.js 8% <0%> (+0.5%) ⬆️
components/drop-zone/provider.js 1.98% <0%> (+0.64%) ⬆️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8621e54...54df7bc. Read the comment docs.

@aduth aduth merged commit b9bdfa2 into master Sep 8, 2017

3 checks passed

codecov/project 32.39% (+1%) compared to 8621e54
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@aduth aduth deleted the add/edit-post-links branch Sep 8, 2017

aaronjorbin added a commit that referenced this pull request Sep 9, 2017

FIX: Saving in Classic Editor shouldn't redirect to Gutenberg
`get_edit_post_link` is used in `post.php` to redirect when saving. See https://core.trac.wordpress.org/browser/trunk/src/wp-admin/post.php#L190

By checking to see if the referer is the classic editor, modification to the edit post link is preserved.

Introduced in #1797
Fixes #2707

aduth added a commit that referenced this pull request Sep 27, 2017

FIX: Saving in Classic Editor shouldn't redirect to Gutenberg
`get_edit_post_link` is used in `post.php` to redirect when saving. See https://core.trac.wordpress.org/browser/trunk/src/wp-admin/post.php#L190

By checking to see if the referer is the classic editor, modification to the edit post link is preserved.

Introduced in #1797
Fixes #2707
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment