-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Apply block order by Flexbox order property #563
Conversation
Works great! Really cool ✨ |
editor/modes/visual-editor/index.js
Outdated
@@ -10,17 +11,23 @@ import './style.scss'; | |||
import Inserter from 'components/inserter'; | |||
import VisualEditorBlock from './block'; | |||
|
|||
function VisualEditor( { blocks } ) { | |||
function VisualEditor( { blocks, blockOrder } ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
blockOrder
is not being use in this component, shoud we move it the the connect
of the VisualEditorBlock
component?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
blockOrder
is not being use in this component, shoud we move it the theconnect
of theVisualEditorBlock
component?
Yep! And in fact, the order
prop is already specified in block.js
's connect
, so just need to remove this. See f86de14.
This is mindblowing. 💥 |
7b9bb91
to
290314c
Compare
So there appears to be some accessibility implications on this, since tabbing will not respect
To illustrate the second point, try the following steps:
Expected: Focus moved to embed block It's not clear to me why this is; I'm wondering if TinyMCE is hijacking the focus change. In my minimal proof of concept, it seems like tabbing should respect |
Forgive me for asking what I know will sound somewhat silly; what if as soon as you press tab, after having rearranged boxes with the arrows, we actually reorder the markup and allow the repaint to happen? This is based on the assumption that people won't necessarily be tabbing around that often, and when they do a single repaint in the edgecases where necessary will be an okay tradeoff. |
I don't know for certain that we have access to control the tabindex behavior on the fly like this, since it sounds like we'd need to intercept the tab key event, rearrange the DOM, then allow the key event to proceed with its default behavior. |
290314c
to
693cc8c
Compare
693cc8c
to
1888ee9
Compare
Closing in response to accessibility concerns outlined at #544 (comment). |
Thanks for trying. |
Fixes #544
This pull request seeks to implement an unfortunate but surprisingly reasonable hack to resolve
iframe
flickering of embed blocks using Flexbox'sorder
property instead of natural DOM ordering. The default behavior of moving an iframe in a DOM results in the iframe being reloaded. The effect here is to never rearrange the DOM position but instead affect ordering through theorder
styling. From a user's perspective, there should be no difference.See also:
Minimal proof-of-concept:
Testing instructions:
Repeat steps to reproduce from #544, verifying that neither moving the embed up nor down results in a flickering reload of the iframe.