[Modal Layout Picker] Create sections and thumbnail UI using static data by chipsnyder · Pull Request #14553 · wordpress-mobile/WordPress-iOS

chipsnyder · 2020-07-31T14:22:31Z

Fixes wordpress-mobile/gutenberg-mobile#2436

To test:

To disable or enable the development version of Modal Layout Picker

Open the app from the build that allows FeatureFlags such as a PR build or a local development build
- By default, the modal layout picker will be enabled.
- From the site page:
  - Click on your Gravatar.
  - Click on App Settings.
  - Click on Debug.
  - Toggle "Gutenberg Modal Layout Picker" to enable or disable the picker
    
    To toggle:

When enabled, the Layout Picker should show when creating a new page from My Site and from the Page list.

Start by navigating to the Modal Layout Picker

General Navigation

Note: Currently, the categories repeat the same layout 30 times because the static data only has one layout per category.

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 an activity indicator indicating it's trying to load
- On first load or when scrolling down for the first time expect to see an activity indicator on images that haven't loaded yet.
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 (no new network calls made for already loaded images).
- Note: Because we're reusing the same image you might see multiple network calls to load the image on the first display. This is expected but you shouldn't see any network requests on subsequent loads. The static data used for the layouts can be found in the Gutenberg project in GutenbergLayouts.swift

Selecting a Layout

Selecting a layout should highlight the cell and add a checkmark
- Open the layout picker and select a cell
iOS 12 and Below iOS 13+ Dark Mode
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" buttons are non-functional right now and will be handled in a later task
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:

iOS 12 and Below	iOS 13+	Dark Mode

Additional Context:

This is a continuation of the work started in #14501 and will be followed up by other tickets as part of Modal Layout Picker Project

Placeholder images and downloading the images on poor network connections will be refined as part of wordpress-mobile/gutenberg-mobile#2445

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.

…ge creation flow

…g/issue/2436-thumbailPreviews

…r more consistent behavior

peril-wordpress-mobile · 2020-07-31T14:22:37Z

You can trigger an installable build for these changes by visiting CircleCI here.

peril-wordpress-mobile · 2020-07-31T14:39:16Z

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

…s as full screen

peril-wordpress-mobile · 2020-07-31T19:48:48Z

	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

chipsnyder · 2020-07-31T20:29:23Z

Hey @iamthomasbishop, I was able to get a build (32382 ) together for you with the category rows in place. Although I do have a couple of design questions for you on it before I call it done.

The footer buttons' border color is a bit hard to see in dark mode with the chrome material. I tried adding more of a weight to it, and that didn't help. Let me know if you have any ideas, and I can try them out and send some screenshots over to you.
On iOS 12 and below I'm using the GridIcons-Checkmark let me know if that works for you. iOS 13+ will use the SF Symbols icon

And of course anything else you notice in this build or that we didn't cover in the last PR.

chipsnyder · 2020-08-03T19:07:22Z

WordPress/Classes/ViewRelated/Gutenberg/Layout Picker/LayoutPickerCategoryTableViewCell.swift

+        let layout = layouts[0] // Static layouts currently only have one layout per category. Reusing the first to help test
+        cell.layout = layout
+        cell.isAccessibilityElement = true
+        cell.accessibilityLabel = layout.title + " \(indexPath.item)"


The accessibilityLabel here is using the indexPath as a temporary crutch for testing since the elements are reused.

Any idea about how descriptive the real names will be?

The names really aren't that descriptive. The title for the majority of them just aligns with the category title, or we could format the slug which is mostly this same pattern category-#. We'll probably want to have a discussion on what the best value for this would be.

chipsnyder · 2020-08-03T19:10:42Z

WordPress/Classes/ViewRelated/Gutenberg/Layout Picker/LayoutPickerCategoryTableViewCell.swift

+    }
+}
+
+class AccessibleCollectionView: UICollectionView {


This class is being used as a way to work around an issue where the collection view wasn't correctly registering the accessible elements when in a table view cell

etoledom · 2020-08-12T12:38:25Z

@etoledom Would you mind reviewing this PR for me as well, I really appreciated your review on the last step of this project?

Sure! Will take a look asap. Probably tomorrow morning 👍

chipsnyder · 2020-08-12T20:02:59Z

WordPress/Classes/ViewRelated/Gutenberg/Layout Picker/GutenbergLayoutPickerViewController.swift

        }
    }

-    var completion: PageCoordinator.TemplateSelectionCompletion? = nil


This was just moved lower in the section of vars largely just so I would still see it.

chipsnyder · 2020-08-12T20:06:13Z

WordPress/Classes/ViewRelated/Gutenberg/Layout Picker/GutenbergLayoutPickerViewController.swift

-            return largeTitleView.frame.height +
-            midHeaderHeight


Switched these (max|mid|min)HeaderHeight to be stored values, because once I started loading the table view with data I found inconsistency with how the header and footers would layout depending on layout timing. Controlling this value just gave me a better experience.

chipsnyder · 2020-08-13T15:28:23Z

WordPress/Classes/ViewRelated/Gutenberg/Layout Picker/LayoutPickerCollectionViewCell.swift

+    func addShadow() {
+        layer.shadowColor = UIColor.black.cgColor
+
+        let scale = UIScreen.main.scale


I'm dividing by the scale on these values to get closer to what the design specs have however this might be temporary as we continue to refine the shadow in design review. I updated this in the category bar PR to remove this scale and use the same point value.

etoledom

Amazing job @chipsnyder ! 🎉
Just one tinny comment on code, but it looks shinny ✨

Testing steps resulted as expected (except loading indicators) ✅
VoiceOver ✅
RTL layout ✅

Downloading the thumbnails don't work amazingly well right now. That will be adjusted in a later PR (see additional notes above)

We're working on the footer buttons dark mode UI

This cover most of my observations. So I'd expect there not to be a loading indicator, right?

A couple other UI small details:

Probably it would be nice to set the trailing constraint of the close button to the safe area layout. It looks to me it's too far to the right on landscape:
The collection view has a nice left padding but it seems to miss an equal right padding.
I'm kind of missing an alternative way to deselect the selected item. This became specially tricky with VoiceOver. The selected item can get lost from the UI and to find it and deselect it can be difficult. Selecting another item and deselecting it works but not sure if we should consider that as an alternative. No hard feelings on this one though, just a thought.

cc @iamthomasbishop to corroborate UI stuff.

etoledom · 2020-08-14T08:04:51Z

WordPress/Classes/ViewRelated/Gutenberg/Layout Picker/GutenbergLayoutPickerViewController.swift


    func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
-        let cell = UITableViewCell(style: .default, reuseIdentifier: nil)
+        let cell = tableView.dequeueReusableCell(withIdentifier: LayoutPickerSectionTableViewCell.cellReuseIdentifier, for: indexPath) as! LayoutPickerSectionTableViewCell


Could we guard this cast?
I understand this should never fail but it might if we modify this class in the future.

If we do want to crash let's do it explicitly with a fatalError with a descriptive message.

Alternatively we could try to recover instantiating the cell manually and calling an assertionFailure to let us know something is wrong.

Same on the CollectionViewController.

Yeah, that's a good point. I think here a guard would just return an empty Table View cell and a crash would be the result of failing to register the correct xib. So I think I like your first option of using the fatalError better that will help future developers if something goes wrong.

etoledom · 2020-08-14T08:05:45Z

WordPress/Classes/ViewRelated/Gutenberg/Layout Picker/GutenbergLayoutPickerViewController.swift

+        guard UIAccessibility.isVoiceOverRunning, let cellIndexPath = tableView.indexPath(for: cell) else { return }
+        tableView.scrollToRow(at: cellIndexPath, at: .middle, animated: true)


…ll call are referencing the same reuse identifier

chipsnyder · 2020-08-14T17:18:47Z

Thanks for the review @etoledom!

If we do want to crash let's do it explicitly with a fatalError with a descriptive message.

This has been added now.

Probably it would be nice to set the trailing constraint of the close button to the safe area layout. It looks to me it's too far to the right on landscape:

The collection view has a nice left padding but it seems to miss an equal right padding.

Great catch on both of these. This has been adjusted.

I'm kind of missing an alternative way to deselect the selected item. This became specially tricky with VoiceOver

This is the only one I didn't take care of yet. I'm not sure what another mechanism would look like but open to suggestions. This could be something we revisit if when we come up with a good approach.

iamthomasbishop · 2020-08-17T20:48:30Z

So I'd expect there not to be a loading indicator, right?

@etoledom That's correct, we'd just show placeholders in the form of ghost/skeleton loaders.

I'm kind of missing an alternative way to deselect the selected item. This became specially tricky with VoiceOver.

The selected item can get lost from the UI and to find it and deselect it can be difficult.

I'm not exactly sure what you mean by "gets lost", but if it is disappearing, that would be frustrating. Ideally, you'd just be able to tap a selected cell again to de-select it or tap another cell to transfer selection, but if we need an explicit "clear" button of sorts, maybe we can do something to facilitate this case.

Agreed w/ you on all of your other feedback 👍 Thanks for jumping on that stuff quickly, @chipsnyder !

etoledom · 2020-08-18T09:35:43Z

I'm not exactly sure what you mean by "gets lost"

I meant: If you select an item, then continue scrolling horizontally, then scroll some more vertically, it can be challenging to navigate back to find again the selected item, specially for VoiceOver users.

I wouldn't say this is a blocker nor something we must fix at all, but more just as an observation. I don't have a good solution in mind either, so no need to focus about this on this PR.

etoledom

Thank you @chipsnyder for the updates!

Looking great to me 🎉

iamthomasbishop · 2020-08-18T14:48:41Z

I meant: If you select an item, then continue scrolling horizontally, then scroll some more vertically, it can be challenging to navigate back to find again the selected item, specially for VoiceOver users.

@etoledom Ahh, okay, that makes more sense. For what it's worth, one of my early design concepts had a special call-out/section at the top of the list that showed "selected layout", but we decided against it because it took up a lot of space. Perhaps this is something we can keep an eye on and add something similar as part of a future iteration. Thanks for raising this!

chipsnyder · 2020-08-19T14:14:19Z

Checked with @iamthomasbishop merging this piece we'll tackle the lingering design items in later PRs and before enabling this in the wild.

chipsnyder added 15 commits July 14, 2020 09:43

Adds cells for building out the layout picker rows for gutenberg's pa…

74cd29a

…ge creation flow

Merge branch 'gutenberg/issue/2419-MLPContainer-smooth' into gutenber…

53f7938

…g/issue/2436-thumbailPreviews

Merge branch 'gutenberg/issue/2419-MLPContainer-smooth' into gutenber…

e43601b

…g/issue/2436-thumbailPreviews

Add in layout picker categories and layouts using static data

a8c72e8

refine header and footer spacing for category rows

f2fa0cb

Merge branch 'gutenberg/issue/2419-MLPContainer-smooth' into gutenber…

bbe9652

…g/issue/2436-thumbailPreviews

Add static text for new buttons

c263104

Handle deselecting of layout cells on second tap

d20a461

Maintain selection of only one element across rows

45d70d9

Handle toggeling the bottom buttons when a cell is selected

72bbe62

Add support for dynamic text on rows

8475e9e

Preserve max header height for more consitent rotation expereience

7023186

Refactor the snap point calculations to not be computed properties fo…

d434747

…r more consistent behavior

Retain scroll position and begin work on tracking selected item

ac45ce5

Retain selected item and enforce single selection across rows

58596ff

chipsnyder added [Type] Enhancement labels Jul 31, 2020

chipsnyder added 5 commits July 31, 2020 14:14

Move reuse identifiers to be part of the cells they reference

5476623

Adjust initial header height to support devices that treat page sheet…

2195ba1

…s as full screen

Add background to checkmark on layout cells

be9f7e8

Correct single selection issue for layout cells

de3e6d9

Style layout cell for dark mode

cc4907c

Add support for voice over

898133e

chipsnyder commented Aug 3, 2020

View reviewed changes

chipsnyder marked this pull request as ready for review August 11, 2020 13:30

chipsnyder added 3 commits August 11, 2020 13:23

Migrate shadow to Collection view to prevent overlap

0a6d220

Adjust table shadow

1ee18ec

Remove outdated documentation

96bae47

chipsnyder mentioned this pull request Aug 12, 2020

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

Merged

3 tasks

chipsnyder added 5 commits August 12, 2020 12:15

Adjust cell size for collection view

9a64539

Adjust bottom margin on table view footer

1a00dbe

Tweak cell shadow from design review

1bb628e

Add border color to unselected cells

fad484a

Update shadow style from design review

82ca58e

chipsnyder commented Aug 12, 2020

View reviewed changes

chipsnyder commented Aug 13, 2020

View reviewed changes

etoledom reviewed Aug 14, 2020

View reviewed changes

chipsnyder added 4 commits August 14, 2020 10:37

Constrain Close button to safe area

1dec656

Add trailing Inset to collection View cells

f03762a

Add informative message if cells fail to register

cc43860

Update explicit messaging to ensure the logging and dequeueReusableCe…

383a4fc

…ll call are referencing the same reuse identifier

etoledom approved these changes Aug 18, 2020

View reviewed changes

chipsnyder removed the [Status] Needs Design Review label Aug 19, 2020

chipsnyder merged commit 14cdfb9 into develop Aug 19, 2020

chipsnyder deleted the gutenberg/issue/2436-thumbailPreviews branch August 19, 2020 14:14

momo-ozawa added Offline Mode (Sync Issues) and removed Offline Mode (Sync Issues) labels Apr 11, 2024

		guard UIAccessibility.isVoiceOverRunning, let cellIndexPath = tableView.indexPath(for: cell) else { return }
		tableView.scrollToRow(at: cellIndexPath, at: .middle, animated: true)

Conversation

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

Uh oh!

To test:

General Navigation

Preview Images

Selecting a Layout

Screenshots:

Additional Context:

PR submission checklist:

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!

chipsnyder commented Jul 31, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

etoledom commented Aug 12, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

etoledom left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chipsnyder commented Aug 14, 2020

Uh oh!

iamthomasbishop commented Aug 17, 2020

Uh oh!

etoledom commented Aug 18, 2020

Uh oh!

etoledom left a comment

Choose a reason for hiding this comment

Uh oh!

iamthomasbishop commented Aug 18, 2020

Uh oh!

chipsnyder commented Aug 19, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

chipsnyder 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

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

edited

Loading