Skip to content

[Modal Layout Picker] Add support for Self-Hosted sites#13017

Merged
antonis merged 8 commits intodevelopfrom
issue/2455-MLP_SelfHosted
Oct 13, 2020
Merged

[Modal Layout Picker] Add support for Self-Hosted sites#13017
antonis merged 8 commits intodevelopfrom
issue/2455-MLP_SelfHosted

Conversation

@antonis
Copy link
Copy Markdown
Contributor

@antonis antonis commented Sep 25, 2020 •
edited
Loading

Fixes: wordpress-mobile/gutenberg-mobile#2455

Related PRs:

Description

This PR populates the MLP UI for self-hosted sites with data fetched from /wpcom/v2/common-block-layouts.

To test:

Layout Picker should show when creating a new page from My Site or Site Pages the Modal Layout Picker appears.

Regular Flow

  1. With a good network connection
  2. Open the app and select a Self-Hosted site (e.g. https://rainy-koala.jurassic.ninja)
  3. Open the Modal Layout Picker
  4. Expect to see a brief ghost of while the data is fetched
  5. Expect to see the screen updated with the fetched layouts
    • Layouts in the filter bar should be in alphabetical order
    • The sections in the table view should match the order of the categories in the filter bar
  6. You should be able to select a layout from a category.

Slow Network

  1. With a slow network connection (e.g. by changing the cellular configuration in an emulator).
  2. Open the app and select a Self-Hosted site
  3. Open the Modal Layout Picker.
  4. Expect to see a ghost of empty cells while the data is fetched.
    • You should not be able to select a cell or category while the data is being fetched.
  5. Expect to see the screen updated with the fetched layouts
    • Layouts in the filter bar should be in alphabetical order
    • The sections in the table view should match the order of the categories in the filter bar
  6. You should be able to select a layout from a category.

No Network

  1. Disable your device's connection ( e.g. airplane mode)
  2. Return to the app and select a Self-Hosted site
  3. Open the Modal Layout Picker.
  4. Expect to see a ghost of empty cells while the data is fetched.
    • You should not be able to select a cell or category while the data is being fetched.
  5. You should be able to still create a blank page
    • 📝We will be using cached data for this flow in the future.

Screenshots:

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@antonis antonis self-assigned this Sep 25, 2020
@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Sep 25, 2020 •
edited
Loading

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@antonis antonis mentioned this pull request Sep 25, 2020
Merged
@antonis antonis requested review from ashiagr, chipsnyder, develric, malinajirka and oguzkocer and removed request for develric September 25, 2020 14:01
@antonis antonis added this to the 15.9 milestone Sep 25, 2020
@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Sep 25, 2020 •
edited
Loading

You can test the changes on this Pull Request by downloading the APK here.

@antonis antonis changed the base branch from develop to issue/2446-MLP_CreatePage September 25, 2020 14:13
@antonis antonis marked this pull request as draft September 25, 2020 14:19
@antonis antonis marked this pull request as ready for review September 25, 2020 16:17
@antonis antonis mentioned this pull request Sep 25, 2020
Merged
3 tasks
Base automatically changed from issue/2446-MLP_CreatePage to develop September 29, 2020 06:10
@antonis antonis modified the milestones: 15.9, 16.0 Oct 4, 2020
@ashiagr ashiagr self-assigned this Oct 5, 2020
@ashiagr
Copy link
Copy Markdown
Contributor

ashiagr commented Oct 5, 2020

Changes looks good @antonis 👍.

Just mentioning two issues (might not require any action)

  1. Some of the categories are translated in the top bar (for a non-English language) but layout titles are still shown in English.

translation

  1. On selecting a layout (e.g. Team) page and press back, app sometimes becomes unresponsive and crashes

crash

Seems like it is a known issue on debug builds:

java.lang.RuntimeException: Debug build crash: Post had changes that needed to be saved when exiting the editor, and there was NOT an autosave pending. This means that the editor's autosave mechanism failed. This only crashes on debug builds.
        at org.wordpress.android.ui.posts.EditPostActivity.lambda$updateAndSavePostAsyncOnEditorExit$16$EditPostActivity(EditPostActivity.java:1662)

Thought to mention since it happened consistently for the "Team" layout.

@antonis
Copy link
Copy Markdown
Contributor Author

antonis commented Oct 6, 2020

Hello @ashiagr 👋 ,

  1. Some of the categories are translated in the top bar (for a non-English language) but layout titles are still shown in English.

Since all content is returned with the same API call I guess this is a data issue. I will check with @enejb and @chipsnyder on this.

  1. On selecting a layout (e.g. Team) page and press back, app sometimes becomes unresponsive and crashes

I'll investigate further if I can handle this on my side. It seems to be related with the updateAndSavePostAsyncOnEditorExit function

Thanks again for your feedback 🙇

@malinajirka
Copy link
Copy Markdown
Contributor

malinajirka commented Oct 6, 2020

I'll investigate further if I can handle this on my side. It seems to be related with the updateAndSavePostAsyncOnEditorExit function

@antonis This is expected behavior and the app crashes only in debug mode.

@antonis antonis requested review from aforcier, develric and planarvoid and removed request for oguzkocer October 7, 2020 05:41
@malinajirka
Copy link
Copy Markdown
Contributor

malinajirka commented Oct 8, 2020

I'm sorry for the confusion, but I mystified you.

When the message says Post had changes that needed to be saved when exiting the editor, but there was an autosave pending. than it’s a known issue and we should just ignore it.

When the message says Post had changes that needed to be saved when exiting the editor, and there was NOT an autosave pending. This means that the editor’s autosave mechanism failed. It’s an unknown issue and we should investigate what’s wrong.

So in our current case it’s not ok and we should investigate why gutenberg didn’t let us (EditPostActivity) know that the content of the post has changed.

@antonis antonis mentioned this pull request Oct 9, 2020
Closed
@antonis
Copy link
Copy Markdown
Contributor Author

antonis commented Oct 9, 2020

Hello @malinajirka , @ashiagr 👋
I opened a GB-Mobile issue on this at wordpress-mobile/gutenberg-mobile#2701

chipsnyder
chipsnyder approved these changes Oct 12, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@chipsnyder chipsnyder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested this out with the included build and everything is working well 👍

@antonis antonis merged commit 58ef4a1 into develop Oct 13, 2020
@antonis antonis deleted the issue/2455-MLP_SelfHosted branch October 13, 2020 05:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@malinajirka malinajirka Awaiting requested review from malinajirka

@ashiagr ashiagr Awaiting requested review from ashiagr

@aforcier aforcier Awaiting requested review from aforcier

@planarvoid planarvoid Awaiting requested review from planarvoid

@develric develric Awaiting requested review from develric

1 more reviewer

@chipsnyder chipsnyder chipsnyder approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@antonis antonis

@ashiagr ashiagr

Labels

[Type] Enhancement

Projects

None yet

Milestone

16.0

Development

Successfully merging this pull request may close these issues.

4 participants

@antonis @ashiagr @malinajirka @chipsnyder