Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 3 years ago

#45037 closed task (blessed) (fixed)

Improve the WordPress Post editing experience

Reported by: jorbin's profile jorbin Owned by: pento's profile pento
Milestone: 5.0 Priority: highest omg bbq
Severity: normal Version:
Component: Editor Keywords: fixed-5.0
Focuses: Cc:

Description

It would be Guten for WordPress to have a more modern editing experience. People on Bergs all over the world would benefit from this. And so would people in lowlands. Block parties could be held in celebration. It would even lay a foundation for a new and improved way for WordPress to be built.

Attachments (5)

45037.diffโ€‹ (12.4 KB) - added by pento 5 years ago.
45037.2.diffโ€‹ (12.9 KB) - added by birgire 5 years ago.
45037.3.diffโ€‹ (12.5 KB) - added by pento 5 years ago.
45037.4.diffโ€‹ (12.5 KB) - added by pento 5 years ago.
45037.5.diffโ€‹ (5.5 KB) - added by Frank Klein 5 years ago.

Download all attachments as: .zip

Change History (33)

#1 @johnbillion
5 years ago

  • Keywords needs-patch added
  • Type changed from enhancement to task (blessed)

#2 @kirasong
5 years ago

  • Keywords early added

This ticket was mentioned in โ€‹Slack in #core by jorbin. โ€‹View the logs.


5 years ago

This ticket was mentioned in โ€‹Slack in #core-js by aduth. โ€‹View the logs.


5 years ago

This ticket was mentioned in โ€‹Slack in #core by jorbin. โ€‹View the logs.


5 years ago

#6 @desrosj
5 years ago

  • Keywords needs-unit-tests added

lib/register.php has functions related to initializing the new editor. These should be merged into core.

@pento
5 years ago

#7 @pento
5 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

45037.diffโ€‹ enables the block editor as the default editor.

It requires the patch from #45112 to function, and will throw JS errors (which are annoying, but don't prevent testing) until #43316 and โ€‹GB#6694 are resolved.

#8 @webkitchen
5 years ago

Well done

@birgire
5 years ago

#9 @birgire
5 years ago

45037.2.diffโ€‹ is a little adjustment on 45037.diffโ€‹ that:

  • Adds inline filter documentation for:
    • apply_filters( 'enter_title_here', __( 'Add title', 'gutenberg' ), $post )
    • apply_filters( 'write_your_story', __( 'Write your story', 'gutenberg' ), $post )
    • apply_filters( 'default_page_template_title', __( 'Default template', 'gutenberg' ), 'rest-api' )
  • Removes the 'gutenberg' text domain.
  • Changes @since 3.7.0 to @since 5.0.0
  • Adds space in: // Ensure the global $post remains the same afterAPI data is preloaded.
  • Inline documents the global $editor_styles;

Some other notes on 45037.diffโ€‹ :

  • Would be nice to explain this in more detail: // It hurts to do this.
  • Check if we can use get_avatar_url() instead of:
    $avatar       = get_avatar( $user_id, 64 );
    if ( $avatar ) {
    	if ( preg_match( "|src='([^']+)'|", $avatar, $matches ) ) {
    		$user_details['avatar'] = $matches[1];
    	}
    }
    
  • It's not clear to me how the global $post can be modified here:
// Ensure the global $post remains the same after API data is preloaded.
$backup_global_post = $post;

$preload_data = array_reduce(
	$preload_paths,
	'rest_preload_api_request',
	array()
);

// Restore the global $post as it was before API preloading.
$post = $backup_global_post;
  • Can we move inline CSS style into an existing CSS file: <div id="metaboxes" style="display: none;"> or maybe there's an existing CSS class for the closed state?
  • Both json_encode() and wp_json_encode() used.
  • It uses filter_var(), is that safe to use in WordPress core? See e.g. https://core.trac.wordpress.org/ticket/35274#comment:2
Last edited 5 years ago by birgire (previous) (diff)

#10 @pento
5 years ago

Thank you for the review, @birgire! I'm working on this locally at the moment, but I've implemented all of your adjustments, and addressed your notes. ๐Ÿ™‚

#11 @pento
5 years ago

In 43777:

Block Editor: Add an is_block_editor() method to WP_Screen.

This method allows checking (or setting) whether the block editor is loading on the current screen.

See #45037.

#12 @pento
5 years ago

In 43780:

Block Editor: Add extra body classes when the block editor is loaded.

See #45037.

@pento
5 years ago

#13 @pento
5 years ago

45037.3.diffโ€‹ is the latest version of the patch that enables the block editor. Basic usage should work, the notable exception here is reusable blocks. There are probably other bugs related to the various open tickets. ๐Ÿ™‚

@pento
5 years ago

#14 @pento
5 years ago

45037.4.diffโ€‹ fixes a bug with autosaves. Now that reusable blocks have landed, it should be pretty solid.

#15 follow-up: @birgire
5 years ago

Update: The JS error described here below was fixed in #45158 - I just verified that.


@pento great, thank you for the update.

I tested the latest patch, but I get

The editor has encountered an unexpected error.

It shows three buttons:

Attempt Recovery
Copy Post Text
Copy Error

The buttons didn't give any direct text feedback (a possible future ticket?) when pressing on them.

The "Attempt Recovery" button didn't solved the issue though.

The "Copy Error" button did copy to the clipboard the following error:

TypeError: Cannot read property 'length' of undefined
    at i (http://wp.test/wp-includes/js/dist/date.min.js?ver=5.0-alpha-20181023.101317:1:3777)
    at d (http://wp.test/wp-includes/js/dist/date.min.js?ver=5.0-alpha-20181023.101317:1:4419)
    at http://wp.test/wp-includes/js/dist/editor.min.js?ver=5.0-alpha-20181023.101317:55:210644
    at Ug (http://wp.test/wp-includes/js/dist/vendor/react-dom.min.js?ver=5.0-alpha-20181023.101317:90:242)
    at Vg (http://wp.test/wp-includes/js/dist/vendor/react-dom.min.js?ver=5.0-alpha-20181023.101317:92:379)
    at Kf (http://wp.test/wp-includes/js/dist/vendor/react-dom.min.js?ver=5.0-alpha-20181023.101317:118:136)
    at Lf (http://wp.test/wp-includes/js/dist/vendor/react-dom.min.js?ver=5.0-alpha-20181023.101317:118:472)
    at jc (http://wp.test/wp-includes/js/dist/vendor/react-dom.min.js?ver=5.0-alpha-20181023.101317:127:342)
    at aa (http://wp.test/wp-includes/js/dist/vendor/react-dom.min.js?ver=5.0-alpha-20181023.101317:126:505)
    at ta (http://wp.test/wp-includes/js/dist/vendor/react-dom.min.js?ver=5.0-alpha-20181023.101317:124:298)

and strangely the "Copy Post Text" button did also copy the error but not the post text.

I also noticed with an earlier patch that the date-picker seemed to be missing some CSS?

But I suspect these errors might just be related to my own install, but I post them here just in case :-)

It seems that Gutenberg will make this call:

file_get_contents( 'https://fonts.googleapis.com/css?family=
Libre+Franklin%3A300%2C300i%2C400%2C400i%2C600%2C600i%2C800%2C800i&subset=latin%2Clatin-ext' )

I wonder if site owners are informed about this external call?

This might also be blocked, if the allow_url_fopen support is turned off. The check true == ini_get( 'allow_url_fopen' ) comes also to mind.

Last edited 5 years ago by birgire (previous) (diff)

#16 in reply to: โ†‘ย 15 @pento
5 years ago

Replying to birgire:

I tested the latest patch, but I get

The editor has encountered an unexpected error.

I haven't been able to reproduce this error, can you confirm a few things for me?

  • Did you run npm install and grunt webpack:dev since svn up-ing your 5.0 branch?
  • Is this on new posts, editing posts, or does it only happen occasionally?
  • Have you cleared your browser cache?

It seems that Gutenberg will make this call:

file_get_contents( 'https://fonts.googleapis.com/css?family=
Libre+Franklin%3A300%2C300i%2C400%2C400i%2C600%2C600i%2C800%2C800i&subset=latin%2Clatin-ext' )

I wonder if site owners are informed about this external call?

Libre Franklin is the editor font that Twenty Seventeen uses. I just confirmed that using โ€‹a plugin that disables Google Fonts will also stop this from loading.

This might also be blocked, if the allow_url_fopen support is turned off. The check true == ini_get( 'allow_url_fopen' ) comes also to mind.

Good point, I'll switch it to wp_remote_get() instead.

#17 @pento
5 years ago

In 43815:

Editor: Minor bug fixes.

Change the default layout slightly, and tweak the HTML stored in post_content.

Props matveb, karmatosed, joen, youknowriad, aduth, iseulde, pento, EphoxJames, mkaz, jnylen0, notnownikki, lonelyvegan, chopinbach, njpanderson, mimo84, intronic, westonruter, mcsf, dmsnell, afercia, paulwilde, mitogh, codebykat, mrahmadawais, kopepasah, circlecube, adamsilverstein, timmydcrawford, ephox-mogran, nbachiyski, JDGrimes, laurelfulford, Soean, mapk, sirjonathan, j-falk, ryelle, helen, netweb, lamosty, willybahuaud, maurobringolf, jorbin, spocke, androb, annaharrison, Afraithe, georgeh, matt, melchoyce, nitrajka, sirreal, babbardel, arush, martinlugton, iandunn, oyously, rileybrook, azaozz, folletto, ianstewart, johnpixle, clorith, joedolson, ipstenu, mrwweb, diegoliv, jeffpaul, lukecavanagh, shaunandrews, hugobaeta, jjj, mizejewski, buzztone, jdembowski, webdevmattcrom, GaryJones, jasonagnew, dd32, ieatwebsites, gma992, swissspidy, dixitadusara, ameeker, stagger-lee, jblz, nicbertino, rahmohn, vladanost, gziolo, lancewillett, lynneux, betsela, fuyuko, msdesign21, thrijith, chanthaboune, Cloud887, hblackett, vishalkakadiya, c-shultz, nfmohit, noisysocks, omarreiss, hedgefield, hideokamoto, mirucon, nosolosw, DannyCooper, burhandodhy, zebulan, benjamin_zekavica, danielbachhuber, jorgefilipecosta, ajitbohra, bph, ChrisVanPatten, dixitadusara, antpb, mikehaydon, jahvi, floriansimeth, Mathiu, amedina, diegoreymendez, etoledom, caxco93, yoavf, welcher, bobbingwide, jonsurrell, notlaura, ocean90, eliorivero, wpscholar, Shelob9, travislopes, earnjam, designsimply, johnwatkins0, dfangstrom, igorsch, jaswrks, daniloercoli, rianrietveld, dimadin, SergioEstevao, dlocc, tinkerbelly, schlessera, sumobi, kjellr, ireneyoast, hypest, tfrommen, intronic, johnny5, samikeijonen, bpayton, atimmer, Rahmon, tg-ephox, nerrad, talldan, Xyfi.

See #45037.

#18 @pento
5 years ago

  • Keywords fixed-5.0 added

Marking this one as fixed, please open new tickets to report bugs.

#19 @pento
5 years ago

In 43816:

Block Editor: Remove some errant debugging included in [43815].

Props bordoni.
See #45037.

This ticket was mentioned in โ€‹Slack in #core-privacy by birgire. โ€‹View the logs.


5 years ago

#21 @Frank Klein
5 years ago

  • Keywords fixed-5.0 removed

Reopening this, because we cannot add a is_block_editor() method that is both a setter, and a getter.It goes against long established WordPress conventions, and fundamental rules of API design.

Using set_is_block_editor() or get_is_block_editor() keeps in line with the original method name. set_uses_block_editor() or get_uses_block_editor() might be cleaner. The reason being that get_is_ is a doubled prefix. is_ in WordPress is used for helper functions that access a particular piece of data, without offering a setter, so it's not the best naming choice in this case.

#22 @pento
5 years ago

Thank you for the patch, @Frank-Klein.

Given how close we are to 5.0, renaming ::is_block_editor() isn't an option. I'm not particularly fussed whether it acts as a setter or not, which makes me lean towards not changing it.

An alternative option would be to provide an is_block_editor() function (in the style of is_admin(), which returns the value of $GLOBALS['current_screen']->is_block_editor(), but doesn't offer the setter functionality.

Last edited 5 years ago by pento (previous) (diff)

#23 @pento
5 years ago

  • Keywords fixed-5.0 added; early needs-unit-tests has-patch needs-testing removed

Marking this one as fixed. We can follow up in a new ticket if there's demand for a different API.

#24 @pento
5 years ago

In 44130:

Block Editor: Add an is_block_editor() method to WP_Screen.

This method allows checking (or setting) whether the block editor is loading on the current screen.

Merges [43777] from the 5.0 branch to trunk.

See #45037.

#25 @pento
5 years ago

In 44133:

Block Editor: Add extra body classes when the block editor is loaded.

Merges [43780] from the 5.0 branch to trunk.

See #45037.

#26 @pento
5 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 44165:

Editor: Merge some minor bug fixes.

There were some tweaks made to the post editor in WordPress 5.0 that hadn't been merged to trunk, this commit rectifies that.

Merges [43815,43816] from the 5.0 branch to trunk.

Props birgire, jorbin, abdullahramzan, adamsilverstein, mrahmadawais, airathalitov, ajitbohra, schlessera, albertomedina, aldavigdis, alexsanford1, xyfi, nitrajka, afercia, andreiglingeanu, euthelup, aduth, sumobi, anevins, azaozz, androb, andrewserong, kallehauge, nosolosw, apeatling, atimmer, arnaudban, asvinballoo, b-07, benlk, blowery, caxco93, benjamin_zekavica, kau-boy, bernhard-reiter, bcolumbia, bph, boblinthorst, bradyvercher, bpayton, brentswisher, bronsonquick, burhandodhy, icaleb, chouby, ehg, chrisvanpatten, butimnoexpert, christophherr, chriskmnds, claudiosanches, danielbachhuber, mrmadhat, danielhw, danieltj, goldsounds, dfangstrom, daniloercoli, dannycooper, nerrad, dsawardekar, davemoran118, davidherrera, dryanpress, davidsword, davisshaver, dmsnell, dlocc, diegoreymendez, dd32, dency, ocean90, donnapep, chopinbach, electricfeet, eliorivero, sewmyheadon, ericnmurphy, foobar4u, circlecube, fabs_pim, flixos90, floriansimeth, gma992, garrett-eclipse, garyj, pento, doomwaxer, revgeorge, gziolo, bordoni, hardeepasrani, helen, luehrsen, herbmiller, toro_unit, ianbelanger, iandunn, igorsch, ipstenu, ireneyoast, israelshmueli, sisanu, jd55, copons, jnylen0, jamestryon, ephoxjames, jamiehalvorson, jsnajdr, jagnew, dciso, octalmage, vengisss, jhoffm34, shenkj, audrasjb, jblz, jeremyfelt, motleydev, jipmoors, sephsekla, joemaller, joemcgill, joen, johndyer, johnjamesjacoby, joshuawold, johnny5, desrosj, jonsurrell, belcherj, sirjonathan, koke, jorgefilipecosta, shelob9, jvisick77, julienmelissas, kopepasah, kadamwhite, codebykat, ryelle, gwwar, kevinwhoffman, coderkevin, ixkaito, kjellr, obenland, lancewillett, postphotos, loicblascos, lucasstark, luigipulcini, lucaskowalski, lukepettway, mahdiyazdani, mahmoudsaeed, mkaz, tyxla, markjaquith, mapk, vindl, m-e-h, mboynes, mattheu, lonelyvegan, mtias, napy84, maurobringolf, maximebj, mayukojpn, woodent, michaelhull, mmtr86, stubgo, simison, mihai2u, mike-haydon-swo, mnelson4, mpheasant, mikeschroder, idpokute, mikeyarce, dimadin, gonzomir, milesdelliott, warmarks, munirkamal, nfmohit, nateconley, greatislander, njpanderson, notnownikki, nielslange, nikschavan, potbot, nshki, webmandesign, oskosk, pglewis, pareshradadiya-1, swissspidy, pbearne, pauldechov, psealock, paulstonier, paulwilde, pedromendonca, ptasker, peterwilsoncc, tyrannous, strategio, piersb, delawski, prtksxna, presskopp, rachelbaker, rachelmcr, rakshans1, rahmon, lamosty, youknowriad, riddhiehta02, noisysocks, deviodigital, sanchothefat, robertsky, _dorsvenabili, rohittm, magicroundabout, rmccue, welcher, ryo511, sagarprajapati, samikeijonen, scottmweaver, kluny, sharaz, giventofly76, designsimply, sstoqnov, hypest, netweb, stevehenty, stuartfeldt, sergioestevao, soean, tammie_l, karmatosed, thrijith, timgardner, timmydcrawford, tjnowell, mirucon, travislopes, truongwp, tjfunke001, vishalkakadiya, vtrpldn, walterebert, westonruter, skorasaurus, somtijds, earnjam, williampatton, willybahuaud, yoavf, zebulan, ziyaddin, abhijitrakas, andreamiddleton, csabotta, dixitadusara, etoledom, faishal, hideokamoto, imath, iseulde, j-falk, jaswrks, johnwatkins0, jomurgel, notlaura, leahkoerper, mcsf, meetjey, 0mirka00, mitogh, ephoxmogran, tacrapo, mzorz, nagayama, omarreiss, ramonopoly, rileybrook, tinkerbelly, shaileesheth, sikander, ssousa, tfrommen, yahil.

Fixes #45037.

#27 @pento
5 years ago

In 44726:

Admin: Re-add some validation from [44048] that was accidentally removed in [44165].

Props david.binda.
See #45037.

This ticket was mentioned in โ€‹Slack in #core by timothybjacobs. โ€‹View the logs.


3 years ago

Note: See TracTickets for help on using tickets.