Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create Modal Layout Picker Container for new Gutenberg pages and feature flag for development #14456

Closed
wants to merge 21 commits into from

Conversation

chipsnyder
Copy link
Contributor

@chipsnyder chipsnyder commented Jul 10, 2020

Fixes: wordpress-mobile/gutenberg-mobile#2419

Related PRs:

Description

Adds feature flags and a container view for creating a new modal layout

To test:

To disable or enable the development version of Modal Layout Picker

Classic Editor and Gutenberg Editor with Modal Layout Picker Disabled.

Note: Modal Layout Picker should be disabled and uneditable in any Release build

  • Create a new page from My Site
    • Create a new page by clicking on the Floating Toolbar Button on the My Site Page then select "Site Page"
    • Expect to see the page editor without being prompted with the new modal layout picker
  • Create a new page from Pages
    • Create a new page by clicking on the ➕ on the Pages screen
    • Expect to see the page editor without being prompted with the new modal layout picker

Gutenberg Editor with Modal Layout Picker Enabled

  • Create a new page from My Site
    • Create a new page by clicking on the Floating Toolbar Button on the My Site Page then select "Site Page"
    • Expect to see the modal layout picker container
    • Select "Create Blank Page"
    • Expect to see the page editor
  • Create a new page from Pages.
    • Create a new page by clicking on the ➕ icon on the Pages screen.
    • Expect to see the modal layout picker container
    • Select "Create Blank Page"
    • Expect to see the page editor

On the Modal Layout Picker

  • Selecting the ✖️icon in the top right-hand corner should close the modal without creating a page.
  • Swiping down around the "Grip" in the top middle of the modal should close the modal without creating a page.
    Dismissing the modal
  • Scrolling up in the stubbed rows should collapse the header. Collapsing the header should:
    • Minimize the title
    • Hide the prompt
    • Leave space for adding the category bar in the future.
  • Scrolling down in the stubbed rows should reset the header when reaching the top.
    Continuing to scroll down after reaching the top should dismiss the modal
  • Toggle Dark mode and check the style seems consistent with other views

Screenshots:

Container Collapsed Header Dark Mode

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 chipsnyder added [Status] Needs Design Review A designer needs to sign off on the implemented design. Gutenberg compatibility labels Jul 10, 2020
@chipsnyder chipsnyder added this to the 15.4 milestone Jul 10, 2020
@peril-wordpress-mobile
Copy link

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

Generated by 🚫 dangerJS

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Jul 10, 2020

You can test the changes on this Pull Request by downloading it from AppCenter here with build number: 31847. IPA is available here. If you need access to this, you can ask a maintainer to add you.

@chipsnyder
Copy link
Contributor Author

Hey @iamthomasbishop
When working on this I also took some time to work with the collapsable header. When adjusting how it collapses I was able to configure how it worked a bit by making it a "slower" scroll and by making it more "snappy." I kind of liked both but wanted to see your feelings. I created a debug toggle to allow you to play with each. When you get some time will you take a look and see if you prefer one or the other.

Toggle "Snappy" collapse

I think on the "slower" scroll I would want it to set into fully open or fully collapsed at the end of the scroll but let me know.

Here are a few GIFs but I think it's hard to tell the difference with out "feeling" it.

slow snappy

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Jul 10, 2020

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

@iamthomasbishop
Copy link

Nice work @chipsnyder — really cool to see this in action. The header transition feels a bit awkward, both with the toggle on and off. I think I would expect that the header would scroll 1:1 with the page and then become fixed once the large title is near the top of the sheet.

Do we by any chance have access to the “standard” navigation bar transition that you see on most of Apple’s apps that have large titles, (a couple of examples: Apple Music, Notes, Mail)? If so, can we just follow that behavior?

@chipsnyder
Copy link
Contributor Author

The header transition feels a bit awkward, both with the toggle on and off.

Yeah, I agree they both could use some refinement to feel just right.

Do we by any chance have access to the “standard” navigation bar transition that you see on most of Apple’s apps that have large titles, (a couple of examples: Apple Music, Notes, Mail)? If so, can we just follow that behavior?

Their transition is built into the navigation bar, so if we used that directly, our title would be left-aligned, and the subtitle or "prompt" would be above the title.

That can give me more of a direction to go in for the refinement. When in large title mode and scrolling up the standard behavior with the title is:

  • The text scrolls "under" where the "collapsed" header would be.
  • After the text has scrolled away completely the smaller font title appears
  • As soon as the text starts to appear again, the smaller font title disappears.

For mimicking that behavior with a prompt text below the title (which Apple gets around by forcing it over the title), I feel like we would follow the same behavior by showing the smaller font title and then just scroll the subtitle off.

Let me know if that doesn't sound like what you'd want but I can adjust my implementation to mirror this and make a new build for you to play with.

@iamthomasbishop
Copy link

Their transition is built into the navigation bar, so if we used that directly, our title would be left-aligned

Ok, that makes sense about the navigation bar, and I can live with left-aligning the title label.

the subtitle or "prompt" would be above the title.

Not quite sure what you mean by "above" here. Do you mean it'd be above the navigation bar on the z-index? Wouldn't the subtitle be on the main scrollable surface with the rest of the content? I think that's the behavior that you're referring to here 👇 , correct?

For mimicking that behavior with a prompt text below the title (which Apple gets around by forcing it over the title), I feel like we would follow the same behavior by showing the smaller font title and then just scroll the subtitle off.

@chipsnyder
Copy link
Contributor Author

Not quite sure what you mean by "above" here. Do you mean it'd be above the navigation bar on the z-index?

We discussed this in another thread but wanted to capture it here in case others had the same question. I actually meant on the y-axis so using the native controls we'd have:

Large title Small title

Wouldn't the subtitle be on the main scrollable surface with the rest of the content?

The way it was in the build you originally played with I had everything in the scrollable area. I do think there is some room with the not "snappy" version to get more of that 1:1 scroll behavior you mention.

I put together a build that has the native controls though so you can get a feel for that. With this approach though I cut out the "sticky" category bar because adding that in results in a need to handle the original question of "how do we want the scroll to feel." This build does have some UI work to do (like the separator bar on the navigation controller should probably be hidden in large title mode). Personally I think we can get a better transition by not using the native controls but I'll leave that up to what you think.

Here's the updated build with the default UI components:
https://appcenter-filemanagement-distrib2ede6f06e.azureedge.net/e1b0452b-bd8d-43ba-b817-769a0120cd27/WordPress%20Alpha.ipa?sv=2019-02-02&sr=c&sig=llPOowJ2wFB9j4wlTRLfd%2FdovXsnvbCej9cE023OB5s%3D&se=2020-07-15T20%3A01%3A41Z&sp=r

@chipsnyder chipsnyder force-pushed the gutenberg/issue/2419-MLPContainer branch from 522fc2b to b52dcdf Compare July 17, 2020 13:39
@iamthomasbishop
Copy link

For transparency, as mentioned in DM here are some notes from a review I did over the weekend:

The layout is feeling a lot better already. I jotted down a few notes as I tested the build, but I think it’s getting pretty close.

Navigation Bar

  • The scroll pace when you initially start scrolling up seems a little jumpy — either faster than 1x or it is snapping too quickly. It seems to me like it’s related to the pacing, because if you try scrolling very incrementally, the collapse is gradual. (video)
  • Is it possible to override the default text-alignment of the navigation bar component? If so, let’s center that label. If it’s a pain, let’s leave it left-aligned and change the subtitle to also align left.

Subtitle/Filter Toolbar

  • There is an oddity when you are scrolled down the sheet a bit and quickly swipe down, the title bounces “over” the description (video). Is it possible to get the subtitle and filter bar to continue scrolling with the rest of the container in this case?
  • If we can’t do that, the easiest way to solve this might be to give the Navigation Bar background a system material (Chrome?) so that when it does fly over the top of the description, the translucency allows the content below the title to shine through a little bit.
  • Speaking of system materials, regardless of how we tackle the scrolling issues, we will also want to apply the same system material background to the filter bar, as well as the footer bar.

Close Button

  • Where is our “close” icon on the top right of the sheet coming from? The colors look right, but the “x” feels maybe slightly lighter than what I’m seeing on most system apps. (comparison: WP alpha vs. Pages, other system apps)

@chipsnyder
Copy link
Contributor Author

chipsnyder commented Jul 21, 2020

Thanks again for the feedback on this @iamthomasbishop! I spent yesterday looking into these and here's what I have so far:


Navigation Bar

The scroll pace when you initially start scrolling up seems a little jumpy — either faster than 1x or it is snapping too quickly. It seems to me like it’s related to the pacing, because if you try scrolling very incrementally, the collapse is gradual. (video)

I did some testing on this and tracked down several different suggestions, and combinations from different threads ([1], [2], [3], [4], [4], [5]) unfortunately none of them seemed to work to remove the snappy feeling.

The one solution I could find with using Apple's transition was to switch the presentation from page sheet to full screen; doing this allowed the view to work as expected.

Page sheet Full Screen

Is it possible to override the default text-alignment of the navigation bar component? If so, let’s center that label. If it’s a pain, let’s leave it left-aligned and change the subtitle to also align left.

We can "technically" do this by setting the titlePositionAdjustment, but this is more for making minor horizontal adjustments for font and not for centering the text. So I for maintainability, I would say we should just left-align the subtitle.

Subtitle/Filter Toolbar

There is an oddity when you are scrolled down the sheet a bit and quickly swipe down, the title bounces “over” the description (video). Is it possible to get the subtitle and filter bar to continue scrolling with the rest of the container in this case?

This bounce is something that's the result of not having the scrollable section (the subtitle and category bar) be a part of the navigation controller. I tried a couple of things like making the bar transparent and turning off the bounce on the scroll. Each of those solutions didn't solve it, nor is there a documented flow to prevent this.

Speaking of system materials, regardless of how we tackle the scrolling issues, we will also want to apply the same system material background to the filter bar, as well as the footer bar.

The footer bar does have the transparency on it right now, but it's hard to see with a white background of the cells.

Close Button

Where is our “close” icon on the top right of the sheet coming from? The colors look right, but the “x” feels maybe slightly lighter than what I’m seeing on most system apps. (comparison: WP alpha vs. Pages, other system apps)

I'm currently using cross-small from the GridIcons library. Since SF-Symbols are exposed in iOS 13, we could use that x-mark instead. The icon would just be a bit different on iOS 13 vs. iOS 12.


Overall thoughts

From those pieces of feedback, I think we would be able to solve those design issues if we went back to Build: 31570 and tried to identify the areas where that feels off instead of investing the time in working around some of these bugs with page sheet and large title transitions. Because that way, we would have all of the control in the header instead of trying to manipulate Apple's navbar.

What are your thoughts?

EDIT:
In case that build is no longer available, I create another branch/PR that mirrors the snapshot from that point #14501 (comment)

@chipsnyder
Copy link
Contributor Author

Closing this PR in favor of the approach in #14501

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] Needs Design Review A designer needs to sign off on the implemented design.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Modal Layout Picker] Create Modal window for Modal Layout Picker
2 participants