Skip to content

remove the animation for appbar layout changes#2024

Merged
devoncarew merged 2 commits into
flutter:masterfrom
devoncarew:remove_appbar_animation
Jun 1, 2020
Merged

remove the animation for appbar layout changes#2024
devoncarew merged 2 commits into
flutter:masterfrom
devoncarew:remove_appbar_animation

Conversation

@devoncarew
Copy link
Copy Markdown
Contributor

  • remove the animation for appbar layout changes

This fixes the issue where the memory page would deadlock when running on Flutter desktop. Because of where the animation was defined in the widget hierarchy, it would cause the app screens to rebuild often. There may still be a deadlock in the memory page, but if so we no longer seem to hit it.

Most pages only rebuild once now when switching screens. The memory page may rebuild twice (from brief observation). And - not topical for this PR - it seems to take about 300-400ms to build the frame when switching into the Inspector page; that all seems to be spent in layout.

Copy link
Copy Markdown
Member

@kenzieschmoll kenzieschmoll left a comment

Choose a reason for hiding this comment

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

LGTM with a couple nits


AnimationController appBarAnimation;
CurvedAnimation appBarCurve;
static const Object _appBarTag = 'DevTools AppBar';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why did this change from String to Object

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's used as an Object later on, so I just updated the type to reflect the intend.

preferredSize = isNarrow
? const Size.fromHeight(kToolbarHeight + 40.0)
: const Size.fromHeight(kToolbarHeight);
final animatedAlignment =
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

rename to alignment since it is no longer animated?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

👍

@devoncarew devoncarew merged commit 9b52b2e into flutter:master Jun 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants