-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Block API: Adding a Block API flag to disable the block's HTML mode #3037
Conversation
In my testing, using your instructions, this works for me 👍 👍 |
@@ -34,7 +35,7 @@ function BlockSettingsMenuContent( { mode, uids, isSidebarOpened, onDelete, onTo | |||
> | |||
{ __( 'Settings' ) } | |||
</IconButton> | |||
{ count === 1 && | |||
{ count === 1 && blockType && blockType.supportHTML !== false && |
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.
Is count === 1
redundant here, since blockType
assumes a single selected block anyways?
import { removeBlocks, toggleSidebar, setActivePanel, toggleBlockMode } from '../actions'; | ||
|
||
function BlockSettingsMenuContent( { mode, uids, isSidebarOpened, onDelete, onToggleSidebar, onShowInspector, onToggleMode, onClose } ) { | ||
export function BlockSettingsMenuContent( { blockType, mode, uids, isSidebarOpened, onDelete, onToggleSidebar, onShowInspector, onToggleMode, onClose } ) { |
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.
Should we consider splitting props onto their own lines, since this one is now quite long? Alternatively, would it be sensible to create a separate component solely responsible for deciding whether and how an "Edit as HTML" button should be rendered?
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.
would it be sensible to create a separate component solely responsible for deciding whether and how an "Edit as HTML" button should be rendered?
Sounds like a good idea
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.
Looks good 👍
import { toggleBlockMode } from '../actions'; | ||
|
||
export function BlockModeToggle( { blockType, mode, onToggleMode } ) { | ||
if ( blockType.supportHTML === false ) { |
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.
Unsure how defensive we should be here, i.e. what if blockType
is null, or in getBlock( state, uid ).name
the property accesses on null
? Not likely to ever occur, but would be destructive if it did.
e4ab3d7
to
162c423
Compare
closes #2985
This PR adds a
supportHTML
block type property to remove support for the HTML mode. The idea is to fix this issue before the next release.Notes
We could consider to infer this from blocks returning
null
in the save function but this is not consistent, Some dynamic blocks can decide to provide fallback HTML and some static blocks could returnnull
depending on some attributes.We need to start thinking about unifying all the
support*
flags under a unique property?Testing instructions