[Mobile] Disable Page Template picker if modal layout picker is available (Update for new capabilities mechanism)#24170
Conversation
|
Size Change: 0 B Total Size: 1.15 MB ℹ️ View Unchanged
|
| public enum Capabilities: String { | ||
| case mentions | ||
| case unsupportedBlockEditor | ||
| case modalLayoutPicker |
There was a problem hiding this comment.
If this modal layout picker is a wpcom only feature as we agreed, shouldn't this be handled only by the WPApp's feature flags only?
There was a problem hiding this comment.
This flag is handling hiding the Starter Page Template bar that currently appears on new pages. Since the wpcom feature does the same thing we don't want to prompt with the layout options twice. This just provides a way for the apps to say I've already presented a layout picker.
There was a problem hiding this comment.
In this case, shouldn't we refer to "deactivate Starter Page Template Bar"?
It feels to me that the library shouldn't know about the external interface, but give the option to hide the internal one.
Unless I'm misunderstanding something 🤔
There was a problem hiding this comment.
I'm ok with renaming this. I read it more as the app has a modal layout picker as one of its capabilities, were the deactivate wording to me sounded less like "something I have" and more like a feature flag. Removing the word modal might be better to match your concern of having the interface bleed into this flag.
Although once we complete this work this flag will go away as well as the current SPT so this will be relatively short-lived.
There was a problem hiding this comment.
I read it more as the app has a modal layout picker as one of its capabilities.
Exactly. The library shouldn't need to know what capabilities the app has.
It's a bit more than just renaming, also we would need to invert it's usage here:
In a way that having this capability false, the library won't show the SPT bar (think of: "The capability in the library IS its SPT bar).
Although once we complete this work this flag will go away as well as the current SPT so this will be relatively short-lived.
Oh I didn't know that. Then no need to be so strict about this I guess :)
I'll leave it to you to decide and ✅
etoledom
left a comment
There was a problem hiding this comment.
As we agreed in comments, let's go ahead with this change and not block on details that will be removed soon.
@chipsnyder - Up to you to implement or not the requested changes 👍
|
Thanks @etoledom! Yeah, I agree since this is short-lived I think we can move forward. I'm going to leave it as is so that way we don't need to update the Android side as well. Thanks for taking the time for the review and for the feedback :) |
Fixes: wordpress-mobile/gutenberg-mobile#2419
Related PRs:
gutenberg-mobile: Disable Page Template picker if modal layout picker is available (Update for new capabilities mechanism) wordpress-mobile/gutenberg-mobile#2505WordPress-iOS: Create Modal Layout Picker Container for new Gutenberg pages and feature flag for development wordpress-mobile/WordPress-iOS#14501Description
This capability was originally tested as part of #23872 this just serves as the update to migrate that strategy to be in line with recent changes.
Below is the description and testing steps from the original PR.
Description
This change enables the mobile editor to accept the capability
modalLayoutPickerand based on that conditionally show or hide the Template Picker.Android hasn't started Modal Layout Picker yet so this only needs to
How has this been tested?
To disable or enable the development version of Modal Layout Picker
To toggle:
On iOS using the build options mentioned above
With Gutenberg Modal Layout Picker toggled on:
With Gutenberg Modal Layout Picker toggled on:
With Gutenberg Modal Layout Picker toggled off:
With Gutenberg Modal Layout Picker toggled off:
With Gutenberg Modal Layout Picker toggled on or off:
On Android running a metro server or generating a build
Types of changes
New feature
Checklist: