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

Make the kitchensink button removable from plugins #10964

Merged
merged 8 commits into from Nov 9, 2018

Conversation

Projects
None yet
4 participants
@azaozz
Contributor

azaozz commented Oct 23, 2018

See #10963.

Tested with the code there, also with plugins. Ideally the kitchensink button would be named the same as in the classic editor: wp-adv, but that brings some problems with /js/tinymce/plugins/wordpress.

@azaozz azaozz requested review from aduth and iseulde Oct 23, 2018

@azaozz azaozz added this to the WordPress 5.0 milestone Oct 30, 2018

@azaozz

This comment has been minimized.

Contributor

azaozz commented Oct 30, 2018

@iseulde hmm, or perhaps we should use the old name, wp-adv, to remove any possibility of regressions and tweak the wp-includes/js/tinymce/plugins/wordpress plugin a bit to make it work properly in the classic block. I'll open a corresponding ticket for core.

@azaozz

This comment has been minimized.

Contributor

azaozz commented Nov 2, 2018

This is a regression and a "blocker" for WP 5.0. This PR is the very minimum that can be done to fix it.

Or perhaps we should handle this in core trac? There we can tweak the existing wordpress plugin and change how it handles the wp-adv button to account for the use in the classic block? We can even implement the same way of hiding/showing the extra toolbar(s), by toggling a class on the container div.

@@ -127,6 +127,12 @@ export default class ClassicEdit extends Component {
},
} );
editor.on( 'init', function() {
if ( editor.settings.toolbar1 && editor.settings.toolbar1.indexOf( 'kitchensink' ) === -1 ) {
editor.dom.addClass( ref, 'has-advanced-toolbar' );

This comment has been minimized.

@iseulde

iseulde Nov 2, 2018

Member

What is this doing?

@iseulde

This comment has been minimized.

Member

iseulde commented Nov 2, 2018

Let's do this, it looks good. I'll test in a bit and approve. :)

@iseulde

iseulde approved these changes Nov 2, 2018

azaozz added some commits Nov 2, 2018

Fixed "kitchensink" :)
- Use the `wp_adv` buttton instead of adding a new one.
- "Remember" when toolbars were open.
- Move handling to the `wordpress` plugin.
@azaozz

This comment has been minimized.

Contributor

azaozz commented Nov 2, 2018

Note: need to update the wordpress TinyMCE plugin in core a bit for this to work.

@gziolo gziolo modified the milestones: WordPress 5.0, 4.4, 4.3 Nov 5, 2018

@youknowriad

This comment has been minimized.

Contributor

youknowriad commented Nov 8, 2018

Anything blocking merge here @azaozz @iseulde?

@azaozz

This comment has been minimized.

Contributor

azaozz commented Nov 8, 2018

This went through few changes but "came back" to the initial code. Should be OK to merge to Gutenberg. https://core.trac.wordpress.org/attachment/ticket/45264/45264.3.diff is needed to fix the regression for core.

@youknowriad

This comment has been minimized.

Contributor

youknowriad commented Nov 8, 2018

@iseulde Would you mind taking another look before merge?

@tofumatt tofumatt requested a review from iseulde Nov 8, 2018

@youknowriad

This comment has been minimized.

Contributor

youknowriad commented Nov 9, 2018

Moving forward here. @iseulde is AFK for some days.

@youknowriad youknowriad merged commit b814d88 into master Nov 9, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@youknowriad youknowriad deleted the fix/classic-block-kitchensink-button branch Nov 9, 2018

grey-rsi pushed a commit to OnTheGoSystems/gutenberg that referenced this pull request Nov 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment