Conversation
| get | ||
| @JvmName("setIsFSETheme") | ||
| set | ||
| @Column var galleryWithImageBlocks: Boolean = false |
There was a problem hiding this comment.
This works well and I don't have a problem with this approach, I would like your thoughts on the alternative approach I took for iOS.
I didn't want to add a temporary key to the data model so I stored it in what would be an EditorThemeElement as a new type experimentalFeatures to make space for future experiments as well. My overall goal was I don't want to create multiple DB changes if we do a lot of experiments but the risk is my approach is added complexity if we don't do more experimental features.
FSE is the space where I can see more experiments happening. WDYT?
There was a problem hiding this comment.
This is indeed a good idea @chipsnyder towards future proofing our implementation.
On Android thought it seems that it is a bit tricky to alter the type check in the EditorThemeElement table in order to add the new experimentalFeature type. An alternative would be to drop and recreate the table or create an entirely new table for the experiments. In any case I think this will add more complexity at this point. I would suggest to keep this as is for now and refactor when the need comes up with a future experiment. Wdyt?
There was a problem hiding this comment.
Ah yeah, I see the dilemma there. I agree that can be a better item to rework if need at the next instance of an experimental flag.
…-blocks-editor-settings_v2 # Conflicts: # fluxc/src/main/java/org/wordpress/android/fluxc/persistence/WellSqlConfig.kt
|
You can trigger optional UI/connected tests for these changes by visiting CircleCI here. |
…-blocks-editor-settings_v2
chipsnyder
left a comment
There was a problem hiding this comment.
I wasn't able to get the Gallery v2 to appear on this build also while testing the analytics the gallery key always showed up as false.
I think the gap is that in EditorThemeSqlUtils.kt makeEditorTheme doesn't handle saving the boolean but perhaps there is something else I'm missing.
I tested by generating a self-hosted site and confirmed I get the expect value from the API.
…-blocks-editor-settings_v2 # Conflicts: # fluxc/src/main/java/org/wordpress/android/fluxc/persistence/WellSqlConfig.kt
mkevins
left a comment
There was a problem hiding this comment.
Tested via: wordpress-mobile/WordPress-Android#15134 (review)
Thanks Antonis! LGTM!
Related PRs
WordPress-Android: wordpress-mobile/WordPress-Android#15129gutenberg-mobile: wordpress-mobile/gutenberg-mobile#3793gutenberg: WordPress/gutenberg#33544Description
This PR adds the Gallery v2 Flag to the EditorTheme to be passed down to the Editor
To test
See WordPress/gutenberg#33544