Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
ShaneK
left a comment
ShaneK
left a comment
There was a problem hiding this comment.
Looks good to me! Just a few leftover iOS-only bits and a couple of things to line up before v10 goes out, none of them blocking.
|
|
||
| <ion-content [fullscreen]="true"> | ||
| <ion-header collapse="condense"> | ||
| <ion-header scrollEffect="condense"> |
There was a problem hiding this comment.
The paragraph under this block still says the large title "displays on iOS devices", but with scrollEffect it condenses in every theme now. I think we can just drop "on iOS devices" from it. The React and Vue quickstarts have the same sentence, and I think the Your First App pages have a similar "due to iOS Collapsible Large Title support" line that could lose the "iOS" too. Since this PR removes the iOS-only note from the title page, the tutorials would end up contradicting it otherwise.
| ## Condensed Header {/* #condensed-header */} | ||
|
|
||
| Ionic provides the functionality found in native iOS applications to show a large toolbar title and then collapse it to a small title when scrolling. This can be done by adding two headers, one above the content and one inside of the content, and then setting the `collapse` property to `"condense"` on the header inside of the content. This effect will only apply when the mode is "ios". | ||
| Ionic provides the functionality found in native iOS applications to show a large toolbar title and then collapse it to a small title when scrolling. This can be done by adding two headers, one above the content and one inside of the content, and then setting the `scrollEffect` property to `"condense"` on the header inside of the content. |
There was a problem hiding this comment.
I think the v10 updating guide needs a header/footer entry for this, since switching from collapse to scrollEffect isn't a straight rename. With scrollEffect the condense and fade effects apply on md too, so an app that follows the deprecation warning will suddenly show its large-title header on Android, where md used to hide it. The guide doesn't mention collapse at all right now. A follow-up PR is fine if you'd rather, it'd just be nice to note that in the PR body.
|
|
||
| <ion-content [fullscreen]="true"> | ||
| <ion-header collapse="condense"> | ||
| <ion-header scrollEffect="condense"> |
There was a problem hiding this comment.
The text above this block says it's what the generated file contains, but the starters still generate <ion-header collapse="condense">, so anyone who runs ionic start will see something different from the docs. That's fine today since the starters are still on v9, but I think we'll want a matching change in the starters repo (tabs and blank, for Angular, React and Vue) before v10 ships. Is that tracked anywhere? The React and Vue versions of this page and the quickstarts have the same mismatch.
There was a problem hiding this comment.
It wasn't tracked, it is now internally. The change is in ionic-team/starters#1936 against the new major-10.0 branch, covering list and sidemenu too.
brandyscarney
left a comment
brandyscarney
left a comment
There was a problem hiding this comment.
This looks good! I tested the playgrounds out using a dev build and I only found one issue related to icons but it's an easy fix!
Issue number: internal
scrollEffectreplaces the deprecatedcollapseprop onion-headerandion-footer, and condense and fade now work in every mode, not just iOS. The iOS-only notes andmode="ios"playground pins are removed where the effect no longer needs them.collapse="condense"andcollapse="fade"toscrollEffect: 67 lines across 55 files, including the Title playgrounds and the Angular, React, and Vue tutorialsDev build:
9.0.12-dev.11794527713.1ebd1ea1