[Site Design Revamp] Main View - remove filtering and action buttons #18506

twstokes · 2022-05-04T01:08:04Z

This PR aims to resolve:

Item D subtask of [Site Design Revamp] Main View Updates #18433 - remove filter pills
Item E subtask of [Site Design Revamp] Main View Updates #18433 - remove primary and secondary buttons
Closes [Site Design Revamp] Main View - Tapping a design launches preview #18447
Doesn't show a checkmark when a design is selected, but shows the border. Removes the border when the preview is dismissed.

Before

Portrait	Landscape

After

Portrait	Landscape

iPad Portrait	iPad Landscape

Animated Border

Simulator.Screen.Recording.-.iPhone.13.-.2022-05-04.at.15.19.39.mp4

Observations:

The gray separator no longer appears between the title and the collection. This matches the designs for the project.
Extra spacing between the title and the collection. We should consider tightening the design after (or while) implementing [Site Design Revamp] Main View - Add recommended designs section #18434.
In landscape the collapsed title isn't shown in the nav bar. This is a known issue.

Change	Design

Testing

No filter bar

Start the Site Creation flow
On the Design Screen, expect that there is no filter bar at the top

No action buttons appear upon design selection

Start the Site Creation flow
On the Design Screen, tap a design
Expect that the design is shown in a Preview modal view

Only the border is shown around a selected design thumbnail when tapping

Start the Site Creation flow
Tap on a design
Expect that a border is shown around the design
Expect that no checkmark is shown
Expect that when dismissing the preview, the border disappears (the design is no longer selected)

Page creation isn't affected

From the My Site view, tap the FAB button (blue circle at the bottom right)
Tap "Site page"
Expect a filter bar
Tap one or more categories on the filter bar and expect the page designs to be filtered
Expect that the primary action button at the bottom says "Create Blank Page"
Tap a design
Expect the action buttons at the bottom to now be "Preview" and "Create Page"
Expect that the selected design has a checkmark and border

Regression Notes

Potential unintended areas of impact
- The tests in [Site Design Revamp] Remove preview mode selector from nav bar #18455 should pass
- The checkmark and border that appears when selecting a Page design
- Accessibility
What I did to test those areas of impact (or what existing automated tests I relied on)
- I manually test the steps above
- I manually tested the tests in [Site Design Revamp] Remove preview mode selector from nav bar #18455
  - Any steps with "Select a design and tap the "Preview" button" were replaced with just tapping the design
- I tested the main view with VoiceOver on and found no regressions
What automated tests I added (or what prevented me from doing so)
- None as this altered the view and not much logic.

PR submission checklist:

I have completed the Regression Notes.
I have considered adding unit tests for my changes.
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.

… and have more control over collection view cells.

wpmobilebot · 2022-05-04T01:32:51Z

You can test the changes in WordPress from this Pull Request by:

Clicking here or scanning the QR code below to access App Center
Then installing the build number pr18506-d17c426 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

twstokes · 2022-05-04T01:35:12Z

...s/ViewRelated/Site Creation/Design Selection/SiteDesignContentCollectionViewController.swift

-            primaryActionTitle: createsSite ? TextContent.createSiteButton : TextContent.chooseButton,
-            secondaryActionTitle: TextContent.previewButton
+            // the primary action button is never shown
+            primaryActionTitle: ""


This is the only part where CollapsableHeaderViewController didn't quite support what we needed. We'll never show a primary action, so there's no need for us to supply a title.

wpmobilebot · 2022-05-04T02:38:08Z

You can test the changes in Jetpack from this Pull Request by:

Clicking here or scanning the QR code below to access App Center
Then installing the build number pr18506-d17c426 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

Gio2018

Hey @twstokes , the test cases work. I left one code comment, and have one observation:
when selecting a design from the design picker, there is no feedback to the user that they have actually selected that design. This is not totally desirable, but can also be addressed in a future PR. If we do so, we should add it here as a known issue.

Gio2018 · 2022-05-04T16:59:28Z

...s/ViewRelated/Site Creation/Design Selection/SiteDesignContentCollectionViewController.swift

-        return sections[sectionIndex].designs[position]
+    var selectedPreviewDevice: PreviewDevice {
+        get { .mobile }
+        set { /* no op */ }


if set does nothing, can we just

var selectedPreviewDevice: PreviewDevice { .mobile }

Good catch! I'll get this fixed. This was left over from being a subclass of FilterableCategoriesViewController which required a setter. 👍

oh I see 👍

This should be resolved with this commit.

twstokes · 2022-05-04T17:35:19Z

when selecting a design from the design picker, there is no feedback to the user that they have actually selected that design

Just to make sure I follow @Gio2018 - are we referring to the time between tapping a design and the preview modal appearing? I agree that we could improve that by adding some sort of animation, though I don't find it super critical since the user will be aware of what they selected after shortly after tapping (assuming the web demo loads!).

Side note: When testing this I found a small bug.

Tap a design, showing the preview
Close the preview
Tap the same design
Nothing happens
Tap the same design again
Preview is shown

It's requiring an extra tap to invisibly "deselect" before firing didSelectItemAt again. I'll get this touched up as well.

Gio2018 · 2022-05-04T17:40:21Z

are we referring to the time between tapping a design and the preview modal appearing?

Yes that's what I was referring to. It isn't super critical and we don't need to block this PR for it, but we should add that improvement at some point, I think.

twstokes · 2022-05-04T17:43:44Z

are we referring to the time between tapping a design and the preview modal appearing?

Yes that's what I was referring to. It isn't super critical and we don't need to block this PR for it, but we should add that improvement at some point, I think.

Ok sounds good - I agree. 👍

…d remove the selection when dismissing the preview view.

twstokes · 2022-05-04T19:17:08Z

The latest commits should address the following:

selectedPreviewDevice is now a constant in SiteDesignContentCollectionViewController
When selecting a design from the Site Design view, a border is shown until its preview is dismissed

I tested for regressions in the Page Design view and didn't find any, though you may notice a slight flicker of the border:

Open the Page Design picker
Tap a design
Tap Preview
Pop the Preview
Observe the border around the picked design - it will flicker

I've confirmed this behavior predates this PR. 😅

twstokes · 2022-05-04T22:01:05Z

I've discovered two more issues that will need fixing before this is reviewed, sorry about that.

Tests are broken with my latest commits
There's an issue where the UITableView in SiteDesignContentCollectionViewController is being told to layout views before they're in the view hierarchy due to fetchDesigns() and the isLoading setter

twstokes · 2022-05-04T22:24:55Z

I've discovered two more issues that will need fixing before this is reviewed, sorry about that.

Tests are broken with my latest commits

There's an issue where the UITableView in SiteDesignContentCollectionViewController is being told to layout views before they're in the view hierarchy due to fetchDesigns() and the isLoading setter

Looks like the UITableView issue has been around a while and predates this PR, so I'll file an issue. 😬 Tests should be fixed - now that selectedPreviewDevice is a constant we were able to remove the "broken" test entirely. 🎉

This is ready for re-re-review. 🙇

twstokes · 2022-05-04T23:37:02Z

Looks like the UITableView issue has been around a while and predates this PR, so I'll file an issue. 😬

Issue filed.

Gio2018

!

twstokes added 4 commits May 3, 2022 16:56

Remove UIPopoverPresentationControllerDelegate conformance.

b707b9c

Add options to cells to disable decorations upon selection.

2240c7b

Remove action button tests now that we don't show action buttons.

8ccca1f

Subclass CollapsableHeaderViewController directly to remote filtering…

3870555

… and have more control over collection view cells.

twstokes added [Type] Task Site Creation labels May 4, 2022

twstokes added this to the 19.9 milestone May 4, 2022

twstokes self-assigned this May 4, 2022

twstokes changed the base branch from trunk to feature/site-design-revamp May 4, 2022 01:08

twstokes mentioned this pull request May 4, 2022

[Site Design Revamp] Main View Updates #18433

Closed

7 tasks

twstokes commented May 4, 2022

View reviewed changes

Add extension to Jetpack target.

abed465

twstokes marked this pull request as ready for review May 4, 2022 01:56

twstokes requested review from a team and Gio2018 May 4, 2022 01:56

twstokes mentioned this pull request May 4, 2022

[Site Design Revamp] Main View - Remove the filter bar #18501

Closed

4 tasks

twstokes changed the title ~~[Site Design Revamp] Main view - remove filtering and action buttons~~ [Site Design Revamp] Main View - remove filtering and action buttons May 4, 2022

Gio2018 reviewed May 4, 2022

View reviewed changes

Make selectedPreviewDevice constant since we're no longer subclassing.

d37f4e4

Only toggle the checkmark for CollapsableHeaderCollectionViewCells an…

8b0e649

…d remove the selection when dismissing the preview view.

twstokes requested a review from Gio2018 May 4, 2022 19:17

Remove unnecessary test now that we're using a constant.

d17c426

Gio2018 approved these changes May 5, 2022

View reviewed changes

twstokes merged commit 48ac178 into feature/site-design-revamp May 5, 2022

twstokes deleted the task/site-design-remove-buttons-filtering branch May 5, 2022 20:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Site Design Revamp] Main View - remove filtering and action buttons #18506

[Site Design Revamp] Main View - remove filtering and action buttons #18506

twstokes commented May 4, 2022 •

edited

Loading

wpmobilebot commented May 4, 2022 •

edited

Loading

twstokes May 4, 2022

wpmobilebot commented May 4, 2022 •

edited

Loading

Gio2018 left a comment

Gio2018 May 4, 2022

twstokes May 4, 2022 •

edited

Loading

Gio2018 May 4, 2022

twstokes May 4, 2022

twstokes commented May 4, 2022 •

edited

Loading

Gio2018 commented May 4, 2022

twstokes commented May 4, 2022

twstokes commented May 4, 2022

twstokes commented May 4, 2022

twstokes commented May 4, 2022

twstokes commented May 4, 2022

Gio2018 left a comment

[Site Design Revamp] Main View - remove filtering and action buttons #18506

[Site Design Revamp] Main View - remove filtering and action buttons #18506

Conversation

twstokes commented May 4, 2022 • edited Loading

Observations:

Testing

Regression Notes

wpmobilebot commented May 4, 2022 • edited Loading

twstokes May 4, 2022

Choose a reason for hiding this comment

wpmobilebot commented May 4, 2022 • edited Loading

Gio2018 left a comment

Choose a reason for hiding this comment

Gio2018 May 4, 2022

Choose a reason for hiding this comment

twstokes May 4, 2022 • edited Loading

Choose a reason for hiding this comment

Gio2018 May 4, 2022

Choose a reason for hiding this comment

twstokes May 4, 2022

Choose a reason for hiding this comment

twstokes commented May 4, 2022 • edited Loading

Gio2018 commented May 4, 2022

twstokes commented May 4, 2022

twstokes commented May 4, 2022

twstokes commented May 4, 2022

twstokes commented May 4, 2022

twstokes commented May 4, 2022

Gio2018 left a comment

Choose a reason for hiding this comment

twstokes commented May 4, 2022 •

edited

Loading

wpmobilebot commented May 4, 2022 •

edited

Loading

wpmobilebot commented May 4, 2022 •

edited

Loading

twstokes May 4, 2022 •

edited

Loading

twstokes commented May 4, 2022 •

edited

Loading