-
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
[RNMobile] Use UnsupportedBlockDetails
component in Missing block
#55133
[RNMobile] Use UnsupportedBlockDetails
component in Missing block
#55133
Conversation
Size Change: +1.66 kB (0%) Total Size: 1.65 MB
ℹ️ View Unchanged
|
I noticed that unit tests related to the Missing block and UBE usage are failing, hence I'm reverting this PR back to draft until I solve them. Seem that those unit tests test against the render tree hierarchy, I'll replace them with integration tests following the testing strategy we applied to other blocks. @dcalhoun I'll let you know when I set the PR back to review, sorry for the noise. |
Flaky tests detected in 404518f. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6434397587
|
These tests have been superseded by integration tests.
@dcalhoun The failures in unit tests have been addressed in the recent commits. It turned out to be a little bit more complex than I expected 😅. The PR is ready to review, let me know if could take a look, thanks 🙇 ! |
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.
packages/block-library/src/missing/test/edit-integration.native.js
Outdated
Show resolved
Hide resolved
Thank you very much @dcalhoun for reviewing the PR 🙇 ! I've just reproduced the case of the duplicated note "Note: You must allow WordPress.com login to edit this block in the mobile editor.", so I'll take a deeper look and solve it. |
I reviewed the different cases that modify the description message in the Jetpack app:
When navigating the code, I recalled that the message is also modified and set empty in third-party blocks (e.g. Jetpack blocks) when using the WordPress app:
@dcalhoun The PR should be ready for a re-review, let me know if you could take a look. Thanks 🙇 ! |
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.
I verified the test plan succeeds on iOS and Android. 🚀
Related PRs:
UnsupportedBlockDetails
component in Missing block wordpress-mobile/gutenberg-mobile#6269What?
Following #54382 (comment), the
UnsupportedBlockDetails
component has been added to the Missing block and replaced the duplicated logic related to the UBE (Unsupported Block Editor).Why?
Reduces code redundancy and allows unsupported blocks to add extra actions.
How?
The logic related to UBE has been removed and the Missing block is now rendering the unsupported block details bottom sheet via
UnsupportedBlockDetails
component.Additionally, a fix
Testing Instructions
Unsupported block can be edited with UBE
Outline is displayed in Classic block
Testing Instructions for Keyboard
N/A
Screenshots or screencast
N/A