[Modal Layout Picker] Create Layout Picker category bar UI using static data by antonis · Pull Request #12573 · wordpress-mobile/WordPress-Android

antonis · 2020-07-31T15:47:42Z

Fixes: wordpress-mobile/gutenberg-mobile#2437

Related PRs:

Depends on: [Modal Layout Picker] Create Layout Picker sections and thumbnail UI using static data #12564

Description

This PR implements the category bar in the header of the Starter Page Template Picker for new pages.

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.

Select a Category

Select a category
- Expect to see the sections condensed to be only the selected category filter
Deselect the category
- Expect to see the other categories return

Select Multiple-Categories

Select a category
- Expect to see the sections condensed to be only the selected category filter
Select a different category
- Expect to see the new section added to the list
Deselect a category
- Expect the list to update again to be only the selected categories
Deselect the rest of the categories
- Expect the all of the categories to return

Selecting a Layout then filter on the same category

Select a layout in a category such as Blog
Select the category in the filter bar that matches the category of the selected layout
- Expect to see the section updated to only show the selected category, and for the selected layout to still be selected.
Deselect the category in the filter
- Expect to see the other categories return, and for the selected layout to still be selected.

Selecting a Layout then filter on the same category deselect layout

Select a layout in a category such as Blog
Select the category in the filter bar that matches the category of the selected layout
- Expect to see the section updated to only show the selected category, and for the selected layout to still be selected.
Deselect the layout
- Expect the bottom bar to revert to "Create a Blank Page"
Deselect the category in the filter
- Expect to see the other categories return, and for no layout to be selected

Selecting a Layout then filter on a different category

Select a layout in a category such as Blog
Select a category in the filter bar that does not match the category of the selected layout
- Expect to see the section updated to only show the selected category, and for the bottom bar to remain as "Preview" and "Create Page"
Deselect the category in the filter
- Expect to see the other categories return, and for the selected layout to still be selected.

Selecting a Layout then filter on a different category select a different layout

Select a layout in a category such as Blog
Select a category in the filter bar that does not match the category of the selected layout
- Expect to see the section updated to only show the selected category, and for the bottom bar to remain as "Preview" and "Create Page"
Select a different layout
Deselect the category in the filter
- Expect to see the other categories return, and for the selected layout from step 3 to still be selected.

Screenshots:

No category	Selected category

No category dark	Selected category dark

Selection	Selection 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 · 2020-07-31T15:47:49Z

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

peril-wordpress-mobile · 2020-07-31T15:54:04Z

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

antonis · 2020-08-03T11:01:40Z

Hello @iamthomasbishop 👋 ,
I have drafted the layout categories selection using static data. There are still some rough edges that I need to take care of (e.g. landscape mode and some UI bugs) but at this point this should behave very close to the final MLP.
You can check the screenshots attached to the PR or download the APK here.

I have a question regarding the expected behavior when the user selected layout is filtered out (Selection dark gif above). Should we just hide it or also clear the user selection?

Thank you in advance for your feedback 🙇

cc @chipsnyder

chipsnyder · 2020-08-03T20:46:04Z

I have a question regarding the expected behavior when the user selected layout is filtered out (Selection dark gif above). Should we just hide it or also clear the user selection?

IMO if the selection is filtered out then we should treat it as being deselected.

iamthomasbishop · 2020-08-04T02:27:16Z

I have a question regarding the expected behavior when the user selected layout is filtered out (Selection dark gif above). Should we just hide it or also clear the user selection?

I have a counter to @chipsnyder's opinion 😃 I would actually keep the selection, because:

If I've tapped a layout and move about, then brought back into view via the filters, its state is stored. In most cases, I want to keep it selected as I explore.
If I want to make a new selection, all I have to do is tap a different one.

Some of the early designs tried a couple of treatments/call-outs for the "selected" one, but in the end decided to keep it super simple and understood the trade-off would be exactly this situation (when filters toggle the row a selected item is in).

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

…tegories

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

…tegories

# Conflicts: # WordPress/src/main/java/org/wordpress/android/ui/mlp/LayoutSelectionListener.kt # WordPress/src/main/java/org/wordpress/android/ui/mlp/LayoutsAdapter.kt # WordPress/src/main/java/org/wordpress/android/ui/mlp/ModalLayoutPickerAdapter.kt # WordPress/src/main/java/org/wordpress/android/ui/mlp/ModalLayoutPickerFragment.kt # WordPress/src/main/java/org/wordpress/android/ui/mlp/ModalLayoutPickerListItem.kt # WordPress/src/main/java/org/wordpress/android/ui/mlp/ModalLayoutPickerViewHolder.kt # WordPress/src/main/java/org/wordpress/android/viewmodel/mlp/ModalLayoutPickerViewModel.kt # WordPress/src/main/res/values/colors.xml # WordPress/src/main/res/values/dimens.xml # WordPress/src/test/java/org/wordpress/android/viewmodel/mlp/ModalLayoutPickerViewModelTest.kt

…mentation

peril-wordpress-mobile · 2020-09-04T10:16:59Z

	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

ashiagr

I tested described flows and everything came across very well. Nice job @antonis !
Just left few code-related comments/ suggestions, lmkwyt.

Also I haven't reviewed test cases yet, will review after this first pass.

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

ashiagr · 2020-09-08T06:11:00Z

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

+        )
+        category.setTextColor(
+                parent.context.getColorFromAttribute(
+                        if (selected) attr.categoriesButtonTextSelected else attr.categoriesButtonText


Can we pass selected state values from the ViewModal itself instead of adding this logic in the ViewHolder?

Also instead of creating our own layouts to filter content, we can use pre-built material filter-chips. These can be added in a ChipGroup and put into a single line using singleLine. This is just a suggestion, feel free to ignore it 🙂.

That's a good suggestion. I've moved the state values to UiState with 52cbeff.

Using the material filter-chips looks like a good improvement. I suggest to leave this to be revisited along with design feedback. Wdyt?

I suggest to leave this to be revisited along with design feedback. Wdyt?

Sounds good 👍

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

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

ashiagr · 2020-09-08T06:43:05Z

WordPress/src/main/res/values-night/styles.xml

+        <item name="categoriesButtonText">@color/white</item>
+        <item name="categoriesButtonBackground">@color/mlp_categories_button_background_dark</item>
+        <item name="categoriesButtonTextSelected">@color/black</item>
+        <item name="categoriesButtonBackgroundSelected">@color/mlp_categories_button_background_selected_dark</item>
+        <item name="categoriesButtonCheckIconColor">@color/black</item>
+        <item name="mlpDividerColor">@color/mlp_divider_dark</item>
+        <item name="categoriesBackground">@color/mlp_categories_background_dark</item>


Instead of using separate colors in values-night/styles.xml and values/styles.xml we can use color attributes from the Material Design theme.

e.g.
For categoriesButtonText which is
<item name="categoriesButtonText">@color/white</item> in dark mode
and
<item name="categoriesButtonText">@color/black</item> in light mode

we can use colorOnSurface which gives the same output and we need not add additional style attributes specially for Modal Layout Picker. Lmkwyt.

I myself get confused when to use new style or use these existing color attributes 🙂 but in the end find that we can eliminate most of such styles.

Fixed with 90dc9c3

Thank you! Wondering if we can eliminate remaining ones as well 🤔. These use different shades of grey:

categoriesBackground categoriesButtonBackground categoriesButtonBackgroundSelected mlpDividerColor

Is it possible that we combine emphasis or alpha values with color attributes like colorSurface to get that shade? We can address it in design review. Feel free to ignore it here.

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

WordPress/src/main/java/org/wordpress/android/viewmodel/mlp/ModalLayoutPickerViewModel.kt

WordPress/src/main/res/layout/modal_layout_picker_fragment.xml

antonis · 2020-09-09T08:49:41Z

Can we add a method onAppBarOffsetChanged in the VM, pass verticalOffset, scrollThreshold as params and call setHeaderTitleVisibility from inside that method based on verticalOffset < scrollThreshold condition?

Yes we can. Thank you for suggesting this :)
I tackled this with 429444d

ashiagr

Changes look good to me. Also I've reviewed the tests.
Thank you for all the hard work here @antonis !

I haven't merged this PR yet because of the "Needs Design Review" label. Feel free to merge if it is not a blocker.

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

antonis self-assigned this Jul 31, 2020

antonis mentioned this pull request Jul 31, 2020

[Modal Layout Picker] Create Layout Picker sections and thumbnail UI using static data #12564

Merged

3 tasks

Category selection UI

fc54662

antonis force-pushed the issue/2437-MLP_Categories branch from 04acaf7 to fc54662 Compare August 3, 2020 09:55

antonis added 2 commits August 3, 2020 13:23

Moved introduced colours to the right place

2381474

Filter layouts by category selection

30c835a

antonis mentioned this pull request Aug 3, 2020

[Modal Layout Picker] Create Modal window for Modal Layout Picker #12482

Merged

3 tasks

antonis added 8 commits August 4, 2020 23:13

Merge branch 'issue/2436-MLP_with_static_data' into issue/2437-MLP_Ca…

35af8c4

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

Merge branch 'issue/2436-MLP_with_static_data' into issue/2437-MLP_Ca…

58466b1

…tegories

Merge branch 'issue/2436-MLP_with_static_data' into issue/2437-MLP_Ca…

9c559e6

…tegories

Merge branch 'issue/2436-MLP_with_static_data' into issue/2437-MLP_Ca…

819f1d6

…tegories

Extracted abstract horizontal RecyclerView view holder functionality

52a6f7e

Merge branch 'issue/2436-MLP_with_static_data' into issue/2437-MLP_Ca…

9214ad8

…tegories

Merge branch 'issue/2436-MLP_with_static_data' into issue/2437-MLP_Ca…

fb0bea9

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

Added tests

6d00a50

chipsnyder mentioned this pull request Aug 24, 2020

[Modal Layout Picker] Create Layout Picker category bar UI using static data wordpress-mobile/WordPress-iOS#14631

Merged

3 tasks

antonis added 4 commits August 25, 2020 08:21

Merge branch 'issue/2436-MLP_with_static_data' into issue/2437-MLP_Ca…

b212382

…tegories

Merge branch 'issue/2436-MLP_with_static_data' into issue/2437-MLP_Ca…

df72853

…tegories

Merge branch 'develop' into issue/2437-MLP_Categories

cf21282

antonis changed the base branch from issue/2436-MLP_with_static_data to develop September 1, 2020 14:19

antonis added 2 commits September 1, 2020 17:33

Extracted categories view holder

b49e050

Added support for selecting multiple categories

f64b5ce

Refactored layout to use CollapsingToolbarLayout and simplified imple…

a95728a

…mentation

antonis added 5 commits September 4, 2020 15:41

Layout fixes and improvements

d12fc3e

Minor view model refactoring

a99bf9d

Removed unnecessary layout file

639accc

Fixed recycler view refresh issues

df974d1

Fixed header visibility issues

426e369

antonis marked this pull request as ready for review September 7, 2020 11:10

antonis requested a review from ashiagr September 7, 2020 11:15

antonis added 2 commits September 7, 2020 14:30

Merge branch 'develop' into issue/2437-MLP_Categories

0df8ae9

Reversed submodule ref change after merge

92f1b4e

ashiagr reviewed Sep 8, 2020

View reviewed changes

antonis added 8 commits September 8, 2020 10:35

Removed unused line of code

b3dd6f9

Moved color attributes logic to uistate

52cbeff

Extracted common parent styles

ad3622c

Removed hardcoded color value

60c6a5c

Used UiHelpers to show the views or make them invisible

ce84160

Moved title visibility logic in the ViewModel

28127ff

Reused existing attributes

90dc9c3

Refactored ViewModel to use a singe UIState

5897942

antonis requested a review from ashiagr September 8, 2020 12:40

antonis added 3 commits September 9, 2020 11:06

Moved logic from viewholder to uistate

8bb5a7a

Renamed VM methods for consistency

2f63777

Moved title visibility logic based on scroll offset in VM

429444d

antonis mentioned this pull request Sep 11, 2020

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

Merged

3 tasks

ashiagr approved these changes Sep 14, 2020

View reviewed changes

Merge branch 'develop' into issue/2437-MLP_Categories

1b33569

antonis merged commit ae6afe7 into develop Sep 14, 2020

antonis deleted the issue/2437-MLP_Categories branch September 14, 2020 06:10

Conversation

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

Uh oh!

Description

To test:

Select a Category

Select Multiple-Categories

Selecting a Layout then filter on the same category

Selecting a Layout then filter on the same category deselect layout

Selecting a Layout then filter on a different category

Selecting a Layout then filter on a different category select a different layout

Screenshots:

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 Aug 3, 2020

Uh oh!

chipsnyder commented Aug 3, 2020

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

ashiagr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ashiagr Sep 8, 2020

Choose a reason for hiding this comment

Uh oh!

antonis Sep 8, 2020

Choose a reason for hiding this comment

Uh oh!

ashiagr Sep 8, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ashiagr Sep 8, 2020

Choose a reason for hiding this comment

Uh oh!

antonis Sep 8, 2020

Choose a reason for hiding this comment

Uh oh!

ashiagr Sep 8, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

antonis commented Sep 9, 2020

Uh oh!

ashiagr 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 31, 2020 •

edited

Loading

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

edited

Loading

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

edited

Loading

iamthomasbishop commented Aug 4, 2020 •

edited

Loading

peril-wordpress-mobile bot commented Sep 4, 2020 •

edited

Loading

ashiagr left a comment •

edited

Loading