Skip to content

[Modal Layout Picker] Populate UI for WPCOM sites with data from the API#12932

Merged
malinajirka merged 15 commits intodevelopfrom
issue/2443-MLP_WPCOM
Sep 23, 2020
Merged

[Modal Layout Picker] Populate UI for WPCOM sites with data from the API#12932
malinajirka merged 15 commits intodevelopfrom
issue/2443-MLP_WPCOM

Conversation

@antonis
Copy link
Copy Markdown
Contributor

@antonis antonis commented Sep 10, 2020 •
edited
Loading

Fixes: wordpress-mobile/gutenberg-mobile#2443

Related PRs:

Description

This PR populates the MLP UI with data fetched from /wpcom/v2/sites/{site}/block-layouts. It also implements a loading state where a view skeleton is displayed and a simple error state.

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 WPCOM site
  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 WPCOM 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 WPCOM 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:

Light Dark
light dark

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.

@peril-wordpress-mobile
Copy link
Copy Markdown

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

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

@peril-wordpress-mobile
Copy link
Copy Markdown

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

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

@antonis antonis mentioned this pull request Sep 11, 2020
Merged
@antonis antonis force-pushed the issue/2443-MLP_WPCOM branch from 2097ba4 to fee7e4c Compare September 11, 2020 12:05
@antonis antonis force-pushed the issue/2443-MLP_WPCOM branch from fee7e4c to c2a1106 Compare September 11, 2020 15:25
@antonis antonis force-pushed the issue/2443-MLP_WPCOM branch from c2a1106 to 0a6f644 Compare September 11, 2020 19:46
@antonis antonis self-assigned this Sep 11, 2020
@antonis antonis changed the title [Modal Layout Picker] Populate UI for WPCOM sites with data from the API [DO NOT MERGE] [Modal Layout Picker] Populate UI for WPCOM sites with data from the API Sep 11, 2020
@antonis antonis mentioned this pull request Sep 11, 2020
Merged
3 tasks
Base automatically changed from issue/2437-MLP_Categories to develop September 14, 2020 06:10
@malinajirka malinajirka self-assigned this Sep 18, 2020
malinajirka
malinajirka reviewed Sep 18, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@malinajirka malinajirka left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks @antonis!

It looks good overall. I've left a couple of in-code comments. I also tested the app and I encounter a couple of scenarios about which I'm not sure about - I think they are expected, but just to be sure.

  1. Should we perhaps always scroll to the top when the user changes a filter? It feels a bit weird when I'm scrolled to the bottom that items appear at the top of the list, wdyt?

  1. I can select layouts when the image hasn't finished loading yet. I think this is fine, just wanted to double check it's expected.

  1. I can select only one layout per category on some of my sites - is this expected?

@peril-wordpress-mobile
Copy link
Copy Markdown

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

Warnings
⚠️ PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS

@antonis
Copy link
Copy Markdown
Contributor Author

antonis commented Sep 22, 2020

Hello @malinajirka 👋 ,

thank you for reviewing this 🙇

  1. Should we perhaps always scroll to the top when the user changes a filter? It feels a bit weird when I'm scrolled to the bottom that items appear at the top of the list, wdyt?

I agree that it feels more natural this way. Fixed with c5c5ebd

  1. I can select layouts when the image hasn't finished loading yet. I think this is fine, just wanted to double check it's expected.

I was thinking of this too, but left it this way since the user may be able to preview the layout even if the thumbnail has not loaded.
Since you feel too that this needs handling I tried to cover it with d012734

  1. I can select only one layout per category on some of my sites - is this expected?

The data are loaded from the server for each site. I guess this is normal but I will double check. Note that in a next PR I will filter out layouts that are not applicable so this "one layout per category" may be a common case.

@antonis antonis requested a review from malinajirka September 22, 2020 10:01
@antonis
Merge branch 'develop' into issue/2443-MLP_WPCOM
74c8750 
# Conflicts:
#	WordPress/src/main/java/org/wordpress/android/ui/mlp/ModalLayoutPickerFragment.kt
@antonis antonis added this to the 15.9 milestone Sep 22, 2020
@malinajirka
Copy link
Copy Markdown
Contributor

malinajirka commented Sep 23, 2020 •
edited
Loading

Thanks @antonis! I've left one comment but the changes look great otherwise. I tried to move the execution to a bg thread, but the app didn't work as expected. So I'll wait for your fixes and test it after. Thanks!

Note that in a next PR I will filter out layouts that are not applicable so this "one layout per category" may be a common case.

We are targeting develop in this PR, is it intentional?

@antonis
Copy link
Copy Markdown
Contributor Author

antonis commented Sep 23, 2020

Hello @malinajirka 👋 ,
thank you for your feedback 🙇

We are targeting develop in this PR, is it intentional?

Yes. This feature is disabled with BuildConfig.MODAL_LAYOUT_PICKER by default (enabled only for wasabi and jalapeno)

malinajirka
malinajirka approved these changes Sep 23, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@malinajirka malinajirka left a comment

Choose a reason for hiding this comment

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

Yes. This feature is disabled with BuildConfig.MODAL_LAYOUT_PICKER by default (enabled only for wasabi and jalapeno)

:D I should wake up before I post comments next time :P.

Thanks for all the changes @antonis! LGTM

@malinajirka malinajirka merged commit 62067f2 into develop Sep 23, 2020
@malinajirka malinajirka deleted the issue/2443-MLP_WPCOM branch September 23, 2020 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@malinajirka malinajirka malinajirka approved these changes

@ashiagr ashiagr Awaiting requested review from ashiagr

@chipsnyder chipsnyder Awaiting requested review from chipsnyder

Assignees

@antonis antonis

@malinajirka malinajirka

Labels

[Type] Enhancement

Projects

None yet

Milestone

15.9

Development

Successfully merging this pull request may close these issues.

2 participants

@antonis @malinajirka