[Modal Layout Picker] Create Layout Picker sections and thumbnail UI using static data by antonis · Pull Request #12564 · wordpress-mobile/WordPress-Android

antonis · 2020-07-30T11:10:28Z

Fixes: wordpress-mobile/gutenberg-mobile#2436

Related PRs:

Depends on: [Modal Layout Picker] Create Modal window for Modal Layout Picker #12482
Followed by: [Modal Layout Picker] Create Layout Picker category bar UI using static data #12573 (and other Modal Layout Picker tasks)

Description

This PR implements the presentation of static layout data as part of the effort to rework the Starter Page Template Picker for new page.
In detail it implements the following:

Layouts grouped into sections based on their category and horizontally scrollable within their category rows.
Preview image on each layout cell
Selecting a Layout:
- Updates the border-color
- Displays the checkmark
- Changes the bottom display to have buttons for "Preview" and "Create Page"
- Deselects any currently selected cells
Deselecting a Layout:
- Reverts the border-color.
- Hides the checkmark.
- Revert the bottom Buttons to be "Create Blank Page"
Handles Dark Mode for the new UI items.

To test:

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

Note: For demo/testing purposes, the categories repeat the same layout 30 times because the static data only has one layout per category.

General Navigation

Category rows should be horizontally scrollable within their sections with a preview image
- Pick a row and scroll horizontally.
- Expect the category to scroll to the end and back again
Scrolling down and back up on the page should retain the scroll position of the category row
- Pick a row and scroll horizontally.
- Scroll vertically.
- Scroll back to your reference row and expect it to be in the same location
Scrolling down and back up on the page should retain the selected layout
- Select a layout
- Scroll vertically and/or horizontally.
- Scroll back to your reference layout and expect it to be selected still

Preview Images

Layouts that haven't loaded yet should display a preview placeholder.
Images should be cached for use offline
- Open the Layout picker and allow some images to download
- Leave the layout picker
- Either go offline or monitor the network calls
- Open the layout picker
- Expect the images for the layout picker to be pulled from the cache

Selecting a Layout

Selecting a layout should highlight the cell and add a checkmark
- Open the layout picker and select a cell
Only one layout should be selected at any given time
- Open the layout picker and select a cell
- Select a different cell (either in the same row or a different row)
- Expect the old cell to deSelect and the new one to be selected
Selecting a Layout should show the buttons "Preview" and "Create Page"
- Open the layout picker and select a cell
- Note: The "Preview" and "Create Page" actions will be addressed by #2452 and #2446
Deselecting a layout should revert the bottom bar to "Create Blank Page"
- Open the layout picker and select a cell
- Deselect the cell by tapping on it again
- Expect the bottom buttons to revert to "Create Blank Page"

Screenshots:

Top scroll	Middle scroll - selection

Top scroll (Dark Mode)	Middle scroll - selection (Dark Mode)

Landscape - selection

Image placeholder	Action

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.

chipsnyder · 2020-07-30T13:45:00Z

WordPress/src/main/java/org/wordpress/android/ui/mlp/GutenbergPageLayouts.kt

+package org.wordpress.android.ui.mlp
+
+//
+// TODO: THIS SHOULD BE MOVED IN RN BRIDGE?


I put it in the bridge for the iOS side that way ideally we can find a way to share these static layouts in the future.

…c_data

peril-wordpress-mobile · 2020-07-31T11:57:05Z

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

peril-wordpress-mobile · 2020-07-31T11:57:05Z

	Warnings
⚠️	PR is not assigned to a milestone.
⚠️	PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS

peril-wordpress-mobile · 2020-07-31T12:05:39Z

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

antonis · 2020-07-31T12:25:13Z

Hello @iamthomasbishop 👋 ,
I have drafted the layout picker sections using static data.
You can test this Pull Request by downloading the APK here.
Thank you in advance for your feedback 🙇

…c_data

chipsnyder

Hey @antonis, great work here too I only found a couple of small issues while testing.

Selection is maintained after closing the view

It seems like the selection is preserved if I leave and come back.

Open the layout picker
Select a layout
Select the up button
Come back
Notice: The layout is still selected.

Scroll position going down and back up

Scroll horizontally in a category (such as the first row)
Scroll to the bottom
Come back to the top
Notice the first row is reset to the default scroll position

Looking at the Google Play store if I do the same thing they keep track of the scroll position. I've noticed other apps like Netflixs will do this as well. What are your thoughts on cloning that behavior?

…c_data # Conflicts: # WordPress/src/main/java/org/wordpress/android/viewmodel/mlp/ModalLayoutPickerViewModel.kt

…c_data # Conflicts: # WordPress/src/main/java/org/wordpress/android/ui/mlp/ModalLayoutPickerFragment.kt

…c_data

antonis · 2020-08-04T20:05:45Z

Hey @chipsnyder 👋 ,
thank you for the feedback. I updated the PR tackling both issues.

…c_data # Conflicts: # WordPress/src/main/java/org/wordpress/android/ui/mlp/ModalLayoutPickerFragment.kt

…c_data

antonis · 2020-08-07T09:28:31Z

WordPress/src/main/java/org/wordpress/android/ui/mlp/GutenbergPageLayouts.kt

+ * NOTE: This implementation will be deprecated and the layouts for self hosted sites will be provided by a new
+ * public API endpoint (/common-block-layouts)
+ */
+object GutenbergPageLayoutFactory {


@chipsnyder My assumption is that this layout factory implementation will be used only temporarily for testing this PR and the new endpoint @enejb is creating will replace it.
Thus there is no need to move this in RN Bridge at this point.

# Conflicts: # WordPress/src/main/java/org/wordpress/android/modules/AppComponent.java

malinajirka

Really great job @antonis!

I've tested the app and it works great. The only thing I noticed is that we lose horizontal scroll position on an orientation change (I'm not sure it's worth fixing though).

I've left some in-code comments. Most of them are very minor. The only one I'd consider a bit more important is the one regarding LayoutSelectionListener - listener implementing lifecycleOwner and livedata seems very strange to me. Let me know what you think ;). Thanks

WordPress/src/main/java/org/wordpress/android/ui/mlp/GutenbergPageLayouts.kt

WordPress/src/main/java/org/wordpress/android/ui/mlp/LayoutsAdapter.kt

WordPress/src/main/java/org/wordpress/android/ui/mlp/ModalLayoutPickerFragment.kt

WordPress/src/main/java/org/wordpress/android/ui/mlp/ModalLayoutPickerListDiffCallback.kt

WordPress/src/main/res/drawable/ic_layout_selection_overlay.xml

WordPress/src/main/res/values/dimens.xml

iamthomasbishop · 2020-08-27T22:24:07Z

@antonis Apologies on the delays for design review, and I hope this is the right place to drop review notes -- if not, let me know and I will drop them in the appropriate place 😄

I've taken the latest build linked above for a spin, and jotted down some notes and feedback. It's feeling pretty close already and I'll give it another run tomorrow, but here are my notes:

I noticed when a row of thumbnails rests below the surface containing the category chips/buttons, I'm unable to interact with or even scroll the categories container -- instead, the thumbnails row is scrolled
Would it be possible to style the sheet in its full-height state with rounded corners, and to hold the same scrim/backdrop style that it has when it's not fully docked? Here's a visual for example.
Can we by any chance replicate the same multi-select capabilities that we're rolling with on iOS?
I'm seeing the large title disappear every once in a while, especially when interacting with the category chips.
The thumbnail rows have a bit too much padding on the bottom, but it sounds like you've sorted that out above.
Would it be possible to apply a smooth transition to the style changes between an un-selected and selected thumbnail image? I believe Material's recommendation for such a transition is an easeOut transition with a 100ms duration, but if we're already using transitions elsewhere natively, it might make more sense to use something that exists.
Can we use the standard elevation shadow on the app bar, even when the category bar is "fixed"
Can we use Noto Serif (bold) for headings?
What font-size is the compact app bar using (when it is "fixed" on scroll)? It feels slightly large, but it might just be my eyes playing tricks, or because I'm used to seeing titles left-aligned on the 72dp keyline. (note: I believe 20sp is the standard size)

Thanks for all of the great work on this thus far! 👏

malinajirka · 2020-08-28T08:12:23Z

@antonis We might want to consider fixing the design-review issues in a separate PR(s). I'm a bit worried it might get out of hands.

This PR is too big from the beginning - we usually create multiple PRs and we merge them into a working branch. When all the PRs are reviewed by developer(s) and ready to go - we ask for a design review. It's way easier to review for developers - each PR has a single focus. And when the whole feature is working the designer can jump in. Another benefit is that we don't have developer and design comments within a single PR as it's a bit difficult to get oriented in for all parties.

antonis · 2020-08-28T08:22:37Z

@antonis We might want to consider fixing the design-review issues in a separate PR(s). I'm a bit worried it might get out of hands.

I agree @malinajirka . I try to cover the technical fixes on this and get it merged. I can follow up on the design fixes separately and fix them in another PR.

iamthomasbishop · 2020-08-28T21:13:06Z

@antonis @malinajirka Totally fine to break this work up as necessary. I was under the impression that this was awaiting design review, so I wanted to share my notes and feedback based on the current state -- to be clear, I don't expect all of the above things I mentioned to be addressed immediately, or even as part of this specific PR. Please feel free to ping me when ready for another round of design review.

malinajirka

Great job! Thanks for all the changes ;)! I've re-reviewed the code and re-tested the changes and all seems to work as expected. The design issues will be fixed in upcoming PRs - I'm merging this PR.

WIP: Basic UI and static demo data

fd2c5f0

antonis added [Status] Needs Design Review A designer needs to sign off on the implemented design. [Type] Enhancement labels Jul 30, 2020

antonis requested a review from chipsnyder July 30, 2020 11:11

WIP: Layout selection

e4fef70

chipsnyder reviewed Jul 30, 2020

View reviewed changes

antonis added 4 commits July 30, 2020 17:41

Merge branch 'issue/2419-MLPContainer' into issue/2436-MLP_with_stati…

23b8a4e

…c_data

WIP: Layout selection

3f6e052

Temporary demo data

2e0f623

Layout selection

4d57409

antonis self-assigned this Jul 31, 2020

Merge branch 'issue/2419-MLPContainer' into issue/2436-MLP_with_stati…

7cfff1c

…c_data

antonis mentioned this pull request Jul 31, 2020

[Modal Layout Picker] Create Layout Picker category bar UI using static data #12573

Merged

3 tasks

chipsnyder reviewed Aug 3, 2020

View reviewed changes

antonis added 7 commits August 4, 2020 14:03

Merge branch 'issue/2419-MLPContainer' into issue/2436-MLP_with_stati…

bc0e05e

…c_data # Conflicts: # WordPress/src/main/java/org/wordpress/android/viewmodel/mlp/ModalLayoutPickerViewModel.kt

Reset user selection on dismiss

381e584

Handle bottom sheet dismissal by user

138d99b

Merge branch 'issue/2419-MLPContainer' into issue/2436-MLP_with_stati…

fc33e06

…c_data # Conflicts: # WordPress/src/main/java/org/wordpress/android/ui/mlp/ModalLayoutPickerFragment.kt

Layouts adapter optimization

49326ce

Merge branch 'issue/2419-MLPContainer' into issue/2436-MLP_with_stati…

030370a

…c_data

Remember scroll position

f448b82

antonis added 4 commits August 5, 2020 09:53

Merge branch 'issue/2419-MLPContainer' into issue/2436-MLP_with_stati…

3f2aca6

…c_data # Conflicts: # WordPress/src/main/java/org/wordpress/android/ui/mlp/ModalLayoutPickerFragment.kt

Merge branch 'issue/2419-MLPContainer' into issue/2436-MLP_with_stati…

11c04d7

…c_data

Merge branch 'issue/2419-MLPContainer' into issue/2436-MLP_with_stati…

7bca27d

…c_data

Merge branch 'issue/2419-MLPContainer' into issue/2436-MLP_with_stati…

81d5c1f

…c_data

Demo data generation

a9f60f2

antonis commented Aug 7, 2020

View reviewed changes

antonis added 3 commits August 7, 2020 14:13

Added layouts recyclerview diff callback

e6e2131

Fixed tests

cdd38c0

Merge branch 'develop' into issue/2436-MLP_with_static_data

94f31c5

antonis marked this pull request as ready for review August 7, 2020 13:57

Merge branch 'develop' into issue/2436-MLP_with_static_data

40fed52

antonis requested review from ashiagr and malinajirka August 24, 2020 12:06

antonis added 2 commits August 25, 2020 08:20

Merge branch 'develop' into issue/2436-MLP_with_static_data

80e9116

Merge branch 'develop' into issue/2436-MLP_with_static_data

31e433e

# Conflicts: # WordPress/src/main/java/org/wordpress/android/modules/AppComponent.java

malinajirka reviewed Aug 27, 2020

View reviewed changes

antonis added 3 commits August 27, 2020 16:51

Renamed method to declare purpose

a31fc15

Renamed vector resource and referenced accent color

605aa7a

Reused existing margins

bfe6a01

Merge branch 'develop' into issue/2436-MLP_with_static_data

3437e30

Extracted ViewHolder in a separate class

5bf4fb5

antonis added 5 commits August 31, 2020 17:06

Moved touch handling to the ui state

8815f40

Removed observer from view holder

05962fa

Removed unneeded checks from RecyclerView DiffCallback

baee77e

Added ui state to track visibility of mlp action buttons

dee9ada

Fixed tests

3f160f7

malinajirka approved these changes Sep 1, 2020

View reviewed changes

malinajirka merged commit 7d32770 into develop Sep 1, 2020

antonis mentioned this pull request Sep 22, 2020

[Modal Layout Picker] Design Review Check wordpress-mobile/gutenberg-mobile#2563

Closed

jkmassel deleted the issue/2436-MLP_with_static_data branch October 17, 2024 18:54

Conversation

antonis commented Jul 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

To test:

General Navigation

Preview Images

Selecting a Layout

Screenshots:

Uh oh!

chipsnyder Jul 30, 2020

Choose a reason for hiding this comment

Uh oh!

peril-wordpress-mobile bot commented Jul 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peril-wordpress-mobile bot commented Jul 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peril-wordpress-mobile bot commented Jul 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

antonis commented Jul 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chipsnyder left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Selection is maintained after closing the view

Scroll position going down and back up

Uh oh!

antonis commented Aug 4, 2020

Uh oh!

antonis Aug 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

malinajirka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

iamthomasbishop commented Aug 27, 2020

Uh oh!

malinajirka commented Aug 28, 2020

Uh oh!

antonis commented Aug 28, 2020

Uh oh!

iamthomasbishop commented Aug 28, 2020

Uh oh!

malinajirka left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

antonis commented Jul 30, 2020 •

edited

Loading

peril-wordpress-mobile bot commented Jul 31, 2020 •

edited

Loading

peril-wordpress-mobile bot commented Jul 31, 2020 •

edited

Loading

peril-wordpress-mobile bot commented Jul 31, 2020 •

edited

Loading

antonis commented Jul 31, 2020 •

edited

Loading

chipsnyder left a comment •

edited

Loading

antonis Aug 7, 2020 •

edited

Loading

malinajirka left a comment •

edited

Loading