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

Restore the help modal in the classic block #11856

Merged
merged 1 commit into from Nov 20, 2018

Conversation

Projects
None yet
6 participants
@azaozz
Contributor

azaozz commented Nov 14, 2018

It has some duplicates with the main help modal, but many things are specific for the classic block. Also, its content can be "tweaked" specifically for the classic block in the wordpress MCE plugin (in core).

#9748

Restore the help modal in the classic block
It has some duplicates with the main help modal, but many things are specific for the classic block. Also, can be "tweaked" specificlly for the classic block in the `wordpress` MCE plugin (in core).

@azaozz azaozz requested review from talldan and iseulde Nov 14, 2018

@azaozz azaozz added this to the WordPress 5.0 milestone Nov 14, 2018

@talldan

This comment has been minimized.

Contributor

talldan commented Nov 15, 2018

Hey @azaozz - The main issue here is that you end up with both modals open at the same time when using the shortcut key from within the classic block:
screen shot 2018-11-15 at 12 03 41 pm

Not sure if there's a way to stop the gutenberg help opening.

I'm also wondering if it might be a bit confusing for users if there are inconsistent modals. If the title of the classic editor modal could be changed to something like 'Classic Block shortcuts' that might help.

@azaozz

This comment has been minimized.

Contributor

azaozz commented Nov 15, 2018

Ah, I see. We should be able to fix this by "capturing" the key event in the wordpress plugin (stopPropagation, etc.). Lets see how we can do that in TinyMCE :)

Actually it may be good to do that for all TinyMCE shortcuts so they don't trigger something "upstream" when an MCE instance is focused.

If the title of the classic editor modal could be changed...

Yep, shouldn't be hard to do that, just need a "sure" way to detect when we are in the classic block instance.

@azaozz

This comment has been minimized.

Contributor

azaozz commented Nov 15, 2018

Hmm, thinking more about this, it would be better to have just one "help modal". How about adding another section to the main Gutenberg modal for the Classic Block? Then can list just the specific shortcuts there, not repeat most of them :)

@talldan

This comment has been minimized.

Contributor

talldan commented Nov 15, 2018

The current Gutenberg modal is getting pretty big, so it might need some thought on how to keep it easily navigable if we add more sections to it.

@azaozz

This comment has been minimized.

Contributor

azaozz commented Nov 15, 2018

True, it's quite long even now :) Would need to look at making it two columns perhaps? Thinking something like that would probably be good?

help-modal-2-col

@mtias

This comment has been minimized.

Contributor

mtias commented Nov 15, 2018

Closes #9748

@karmatosed

This comment has been minimized.

Member

karmatosed commented Nov 16, 2018

I don't think we should add more sections to the existing modal. It is ok to have 2 modals but it's not ok to show them both together. Classic is very specific and unless mistaken as to what is being suggested, it would be weird to see that information in the non-classic modal.

@iseulde

This comment has been minimized.

Member

iseulde commented Nov 16, 2018

We could override the shortcut if the classic block is selected?

@azaozz

This comment has been minimized.

Contributor

azaozz commented Nov 16, 2018

@karmatosed Sure. Still thinking we could try making the "global" help popup responsive, i.e. switch to two columns when the viewport is wide enough, perhaps in v.2?

There may also be other blocks that have specific shortcuts. Thinking we may need an "uniform" way to display them? One thing I tried (just visually) was to move the "button" that opens the global popup to the "More options" menu on the block toolbar. May be nice to have it there, and we can tweak the content according to block type :)

@iseulde yeah, will need to override the shortcut in the Classic Block instance. Was wondering if we need to override all TinyMCE shortcuts so they don't trigger something else "upstream" when an instance is focused. Will also change the title to "Classic Block Keyboard Shortcuts" as discussed above.

@azaozz

This comment has been minimized.

Contributor

azaozz commented Nov 16, 2018

will need to override the shortcut in the Classic Block instance

That means part of the fix for this will be in the wordpress TinyMCE plugin in core. Created https://core.trac.wordpress.org/ticket/45365 for it.

@youknowriad

Let's improve on that on a separate PR

@youknowriad youknowriad merged commit d5cf1d7 into master Nov 20, 2018

1 check passed

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

@youknowriad youknowriad deleted the fix/restore-classic-block-help-modal branch Nov 20, 2018

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

Restore the help modal in the classic block (WordPress#11856)
It has some duplicates with the main help modal, but many things are specific for the classic block. Also, can be "tweaked" specificlly for the classic block in the `wordpress` MCE plugin (in core).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment