[Mobile] - Remove themes from supported endpoints
#63183
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
… with the Android implementation
geriux
added
[Type] Bug
geriux
changed the title
themes from supported endpoints
This was referenced Jul 5, 2024
|
Size Change: +168 B (+0.01%) Total Size: 1.76 MB
ℹ️ View Unchanged
|
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
dcalhoun
approved these changes
Jul 5, 2024
There was a problem hiding this comment.
Thank you for addressing this issue! 🙇🏻
I tested these changes on a physical Samsung Galaxy S20 and iPhone 15 simulator. The test plan succeeded and I did not encounter any issues related to theme colors — both with and without an activated Gutenberg plugin.
- Expect to see the default theme colors in the Color palettes (Text/Background color)
To avoid any confusion, I interpreted this to be mean default colors packaged with WordPress core. I.e., the colors are not from/associated with the active theme. With that understanding, the expectation was met.
After activating the Gutenberg plugin, the theme's color were expectedly then present and available in the block editor.
💡 We might consider adding a change log entry.
Comment on lines
10
to
11
| // Don't add /themes endpoint due to an issue on Android | ||
| // with the wp.org API implementation for React Native. |
There was a problem hiding this comment.
It may be worth publishing a comment on wordpress-mobile/WordPress-Android#21005 capturing the identified issue and linking to it within this code comment. Alternative, inlining an explanation in the code comment would work well too. WDYT?
There was a problem hiding this comment.
Good idea! I'll update it 👍
That's it, the expected colors are the default ones from core 👍
I'll add it now 🚀 Thanks for the review and testing! |
huubl
pushed a commit
to huubl/gutenberg
that referenced
this pull request
Jul 10, 2024
* Mobile - Remove themes from the allowed API endpoints due to an issue with the Android implementation * React Native Editor - Update CHANGELOG * Native API Fetch setup - Update comment for disabling the themes endpoint Co-authored-by: geriux <geriux@git.wordpress.org> Co-authored-by: dcalhoun <dpcalhoun@git.wordpress.org>
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:
themesfrom the allowed API endpoint list wordpress-mobile/gutenberg-mobile#6967What?
Due to recent changes, the editor now fetches the
/themesendpoint when it opens, this is causing an issue on Android devices.Note: The theme endpoint was added in #33654 to be able to show the
Resize for smaller devicesblock option for the Embed block but I haven't been able to see this option with a testing site I have, same on the Web editor.Why?
This is currently affecting self-hosted sites preventing them from using the editor which freezes after attempting to call this endpoint.
The native host apps already fetch the themes data and sends the information the editor needs. It appears there are some issues with how the native editor makes the GET request through the bridge and the host app.
How?
By removing
themesfrom theSUPPORTED_ENDPOINTSlist for now.Testing Instructions
Testing Instructions for Keyboard
Note
Use the following build to test:
Screenshots or screencast
Before-Android.mov
After-Android.mp4