Skip to content

Conversation

@ivirtex
Copy link
Contributor

@ivirtex ivirtex commented May 8, 2022

This is copy of PR #72100 with added tests.
Per #62298, adds magnification for CupertinoSliverNavigationBar large title on over scroll.

Fixes #62298

cupertino nav bar

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

@flutter-dashboard flutter-dashboard bot added f: cupertino flutter/packages/flutter/cupertino repository framework flutter/packages/flutter repository. See also f: labels. labels May 8, 2022
child: Transform.scale(
// This scale is estimated from the settings app in iOS 14.
// It has a maximum magnification of about 15%.
scale: math.min(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LongCatIsLooong would you mind looking at this review? It's the same as https://github.com/flutter/flutter/pull/72100/files#r540634692 except the comment and tests, and the logic went from

math.max(1.0, math.min(1.15, 1 + (constraints.maxHeight - maxExtent) / maxExtent * 0.12))

to

math.min(1.15, 1 + (constraints.maxHeight - maxExtent) / maxExtent * 0.12)

From the other review Daniel got stuck here:

I'm having trouble setting the max-width condition for the scale transformation. My original thought was to have a condition that would check for the width of the large title and ensure it's less than the nav bar builder's constraints.maxWidth, but I can't find a way to get the width of the large title text itself. Any ideas?

And @xster said:

The canonical way is to use a RenderBox the way the FittedBox object uses a RenderBox. During the layout phase, it lets its child calculate its size then reacts accordingly in its own paint.
The slightly less complicated way is to "dry run" it using a TextPainter. There's a bit of that going on in the picker.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems when the scale reaches 1.15 there's nothing preventing a jumbo title from getting clipped?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also when the scroll view isn't overscrolling, it's possible that we have constraints.maxHeight < maxExtent, so clamping is still necessary.

Copy link
Contributor

@LongCatIsLooong LongCatIsLooong May 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think another option is to give the large title widget a modified width constraint, such that newConstraints.maxWith == constraints.maxWidth/1.15, so when it expends it won't exceed the original max width.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

f: cupertino flutter/packages/flutter/cupertino repository framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Strech animation for CupertinoSliverNavigationBar

3 participants