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

Fix: Block toolbar does not appear and movers can not be used on block editor without editor module #15470

Merged
merged 1 commit into from
Jun 18, 2019

Conversation

jorgefilipecosta
Copy link
Member

On the widgets screen, we were not loading the ObserveTyping component, this made actions that change the isTyping flag not being fired as expected, the editors stayed most of the types in an isTyping state = true. This made the block tollbar not appear as expected and made it hard/impossible to use the block movers.

How has this been tested?

I went to /wp-admin/admin.php?page=gutenberg-widgets.
I added some blocks, I verified the block toolbar appears as expected, and I could correctly use the block movers.

@jorgefilipecosta jorgefilipecosta added [Type] Bug An existing feature does not function as intended [Feature] Widgets Screen The block-based screen that replaced widgets.php. labels May 6, 2019
@@ -22,7 +23,9 @@ function WidgetArea( { title, initialOpen } ) {
onInput={ updateBlocks }
onChange={ updateBlocks }
>
<BlockList />
<ObserveTyping>
Copy link
Contributor

Choose a reason for hiding this comment

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

  • What about WritingFlow?
  • This makes me wonder if WritingFlow and ObserveTyping could be included by default in BlockEditorProvider?
  • I think at least ObserveTyping could be part of WritingFlow, Thoughts? @aduth @ellatrix

Copy link
Member

Choose a reason for hiding this comment

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

This makes me wonder if WritingFlow and ObserveTyping could be included by default in BlockEditorProvider?

My problem with it would be more conceptual, i.e. defining what it is that the provider is meant to be doing.

My understanding was that it’s just preparing the state context.

The point of making these things individual components was as an opt-in thing

I’m not opposed to baking some of it into the default behavior, but it does muddy the definition of (a) what the provider is meant to be doing and (b) what baseline to we judge against to decide when and when not to isolate behavioral components

Copy link
Member Author

@jorgefilipecosta jorgefilipecosta Jun 17, 2019

Choose a reason for hiding this comment

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

It looks like we went in the direction of removing functionality from BlockEditorProvider e.g: SlotFillProvider was removed.
I included the writing flow addition.
I guess we can keep this approach where we have granular components, and then we can try to expose a more generic version that includes a block editor with the most common functionality e.g: BlockEditor with SlotFillProvider, ObserveTyping, WrittingFlow etc...
The widget screen would not use this version because it is a complex scenario with multiple editors on the same page and we need some low-level control.

I'm also opened to merging ObserveTyping inside WrittingFlow. @ellatrix would that be a positive improvement? Either way, I guess we can make that change in other PR if this one gets approved first.

@jorgefilipecosta jorgefilipecosta changed the title Fix: Block toolbar does not appear and movers can not be used on widget screen. Fix: Block toolbar does not appear and movers can not be used on block editor without editor module May 30, 2019
@jorgefilipecosta jorgefilipecosta added the Needs Technical Feedback Needs testing from a developer perspective. label May 30, 2019
@jorgefilipecosta jorgefilipecosta force-pushed the add/observe-typing-in-widgets-screen branch from e2d7a86 to f13f817 Compare June 17, 2019 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Widgets Screen The block-based screen that replaced widgets.php. Needs Technical Feedback Needs testing from a developer perspective. [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants