fix: Enable access to block settings within UBE #48435
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The "Show more settings" menu item is no longer included in the block toolbar after #46709 merged. Rather than relying upon that menu item, this conditionally displays the sidebar toggle button whenever a block is selected.
CSS selectors rely upon a single white space between selectors to represent an ancestor relationship. Globally removing white space in the stylesheet breaks this functionality, as it transforms the selector to target a single element with all the selectors. The white space stripping should likely be replaced with a proper CSS minification long term.
Hide the entire "block settings" drop-down menu now that we no longer rely upon it to access the "Show more settings" menu option that was removed entirely.
This relates more to editor behavior than the post content.
dcalhoun
added
[Type] Regression
dcalhoun
added
the
Mobile App - Automation
|
Size Change: -189 B (0%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
|
Hey @geriux. 👋🏻 There is one known issue referenced in the PR description, but I wanted to seek your feedback on the approach here. Also, if you have any ideas on known issue let me know. 😬😄 Thanks! |
|
For the known issue, it appears the logic clearing block selection does dispatch Additionally, I attempted to leverage the It is like this space is a "twilight zone." 😄 Not sure what is going on. |
One thing that noticed from this comment was the mention of the post title and then I realized from testing and seeing the demo video that the Post title and the (+) Button are there. I guess something changed in Gutenberg Web and how it loads the editor but it looks like other things we have for the UBE are not working correctly, like the CSS rules to hide the Post title, (+) button, etc. Maybe if we check why that's not working it could solve the issue you mentioned about the post title clearing the selection, what do you think? I looked for an older screenshot of the UBE and this is what it looked like:
As you can see only the toolbar and the block are visible. |
Yes, I noticed the same and investigated it a bit.
I had considered them separate issues, but you are right that solving #47966 might have an impact on the clear selected block issue. I'll look into it more. 👀 |
Ohh I missed that issue, thanks for sharing the link 😃
I wonder if we could use the iFrame's address to render the content in the UBE directly 🤔 I haven't tested this but maybe it could be a possible fix if trying to inject content within the iframe becomes difficult. |
Nevermind, it is setting the editor's content using |
The editor canvas now relies upon an iframe. It is not possible to style elements within an iframe from the parent context. This copies the styles from the parent conext to the iframe. Additionally, the logic selecting the first block also failed due to the block not existing when it was invoked. This relocates that logic until after the iframe is ready.
On Android, there were times where the iframe was present, but the nested window was not yet ready.
Append the styles to the `document` element, as React will remove the mutation to the `head` element.
…e-access-to-block-settings-within-ube
dcalhoun
commented
Feb 28, 2023
| @@ -327,16 +327,16 @@ private void injectCssScript() { | |||
| mWebView.evaluateJavascript(injectCssScript, message -> { | |||
| if (message != null) { | |||
| String editorStyle = getFileContentFromAssets("gutenberg-web-single-block/editor-style-overrides.css"); | |||
| editorStyle = removeWhiteSpace(removeNewLines(editorStyle)); | |||
There was a problem hiding this comment.
This rudimentary stripping of whitespace inadvertently modifies the CSS selectors, causing them to be interpreted as completely different selectors. E.g. .ancestor .descendent is different from .sibling-1.sibling-2. Ideally, a proper CSS minification is utilized long term.
| // Setup the editor with the inserted block. | ||
| const post = window.wp.data.select( 'core/editor' ).getCurrentPost(); | ||
| window.wp.data | ||
| .dispatch( 'core/editor' ) | ||
| .setupEditor( post, { content: blockHTML } ); | ||
|
|
||
| // Select the first block. | ||
| const clientId = blockEditorSelect.getBlocks()[ 0 ].clientId; |
There was a problem hiding this comment.
This often threw an error where the first block was undefined. This logic was relocated to editor-behavior-overrides to await the iframe render.
Comment on lines
+126
to
+129
| // The editor-canvas iframe relies upon `srcdoc`, which does not trigger a | ||
| // `load` event. Thus, we must poll for the iframe to be ready. | ||
| let overrideAttempts = 0; | ||
| const overrideInterval = setInterval( () => { |
There was a problem hiding this comment.
I explored a few different approaches for awaiting the iframe render. Unfortunately, none of them succeeded.
iframe.onloaddoes not work when theiframeusessrcdocinstead ofsrc.- Awaiting a "ready Gutenberg" via subscribing to the store runs before the
iframerenders.
Comment on lines
+148
to
+150
| // Append to document rather than the head, as React will remove this | ||
| // mutation. | ||
| canvasIframe.contentDocument.documentElement.appendChild( |
There was a problem hiding this comment.
The iframe implementation "manages" the head element. So, we must avoid mutating that element, otherwise React may remove any changes we make.
|
👋🏻 @geriux. I updated this PR to address the missing styles hiding the unrelated editor UI. Unfortunately, doing so did not resolve the issue where tapping certain areas of the do not properly trigger a That said, while I am not too proud of the implementation presented in this PR, I am wondering it makes sense to land this in a beta fix for the time being. It improves the UX a good bit IMO and we are approach the app release. If desired, we could iterate further on the implementation at a future time. For future iterations, there are a lot of oddities that occur due to the new |
I agree we should land this for now and see what we do afterward, I'll focus on reviewing and testing today, thanks!
I was thinking maybe we could add a flag so it doesn't load the iFrame for the UBE, I saw this PR so maybe we could do the same. |
Great — thanks!
Possibly. I did see the flag as well. However, It seems the |
dcalhoun
added a commit
that referenced
this pull request
Mar 1, 2023
* fix: Enable access to block settings within UBE The "Show more settings" menu item is no longer included in the block toolbar after #46709 merged. Rather than relying upon that menu item, this conditionally displays the sidebar toggle button whenever a block is selected. * fix: Disable white space stripping that breaks nested CSS selectors CSS selectors rely upon a single white space between selectors to represent an ancestor relationship. Globally removing white space in the stylesheet breaks this functionality, as it transforms the selector to target a single element with all the selectors. The white space stripping should likely be replaced with a proper CSS minification long term. * fix: Hide block actions unrelated to editing a single block Hide the entire "block settings" drop-down menu now that we no longer rely upon it to access the "Show more settings" menu option that was removed entirely. * refactor: Relocate script toggling block settings visibility This relates more to editor behavior than the post content. * fix: Apply styles and script to editor canvas iframe The editor canvas now relies upon an iframe. It is not possible to style elements within an iframe from the parent context. This copies the styles from the parent conext to the iframe. Additionally, the logic selecting the first block also failed due to the block not existing when it was invoked. This relocates that logic until after the iframe is ready. * fix: Expand conditional checks for partial DOM trees On Android, there were times where the iframe was present, but the nested window was not yet ready. * refactor: Rename for brevity * fix: Avoid React removing appended iframe styles Append the styles to the `document` element, as React will remove the mutation to the `head` element. * docs: Add change log entry
4 tasks
geriux
pushed a commit
that referenced
this pull request
Mar 2, 2023
* Release script: Update react-native-editor version to 1.89.0 * Release script: Update with changes from 'npm run core preios' * Mobile - Update changelog * Release script: Update react-native-editor version to 1.89.1 * Release script: Update with changes from 'npm run core preios' * fix: Enable access to block settings within UBE (#48435) * fix: Enable access to block settings within UBE The "Show more settings" menu item is no longer included in the block toolbar after #46709 merged. Rather than relying upon that menu item, this conditionally displays the sidebar toggle button whenever a block is selected. * fix: Disable white space stripping that breaks nested CSS selectors CSS selectors rely upon a single white space between selectors to represent an ancestor relationship. Globally removing white space in the stylesheet breaks this functionality, as it transforms the selector to target a single element with all the selectors. The white space stripping should likely be replaced with a proper CSS minification long term. * fix: Hide block actions unrelated to editing a single block Hide the entire "block settings" drop-down menu now that we no longer rely upon it to access the "Show more settings" menu option that was removed entirely. * refactor: Relocate script toggling block settings visibility This relates more to editor behavior than the post content. * fix: Apply styles and script to editor canvas iframe The editor canvas now relies upon an iframe. It is not possible to style elements within an iframe from the parent context. This copies the styles from the parent conext to the iframe. Additionally, the logic selecting the first block also failed due to the block not existing when it was invoked. This relocates that logic until after the iframe is ready. * fix: Expand conditional checks for partial DOM trees On Android, there were times where the iframe was present, but the nested window was not yet ready. * refactor: Rename for brevity * fix: Avoid React removing appended iframe styles Append the styles to the `document` element, as React will remove the mutation to the `head` element. * docs: Add change log entry --------- Co-authored-by: Gerardo <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related PRs
What?
Repair access to the block settings sidebar within the unsupported block editor
(UBE). Fixes #48298.
Repair hiding of unrelated editor UI in the UBE. Fixes #47966.
Why?
Without access to the block settings, a user cannot modify important settings
for a given block.
Also, displaying UI elements unrelated to the sole block is
confusing and unnecessary.
How?
Block Settings
Previously, the UBE relied upon the presence of the "Show more settings" button,
but that was removed in #46709. This updates the UBE to rely upon the sidebar
toggle button that is present in the post editor header. Because the sidebar toggle
button can also reveal the post settings, this only displays the sidebar toggle
button whenever a block is selected.
Post Title Visibility
The override styles used by the UBE were no longer successfully applied once #46212
wrapped the editor within an
iframe. It is not possible to styleiframechildren elements from the parent
documentcontext. These changes nocopy the override styles to reside within the
iframedocument so they areapplied to the children elements.
Unrelated Changes
This also updates existing Android-specific scripts to only run when Android
devices are detected by parsing the user agent string. Previously, the entire
editor-behavior-overridesscript was only loaded by the Android platform.Now, both platforms load the script, and portions of it only run for Android.
Testing Instructions
Verify the steps outlined in #48298, #34668, and #47966 pass.
e.g. Archives, Table.
Testing Instructions for Keyboard
n/a
Screenshots or screencast
Screen.Recording.2023-02-27.at.20.45.31.mov