Skip to content

Conversation

@gspencergoog
Copy link
Contributor

@gspencergoog gspencergoog commented Oct 8, 2019

Description

This attempts to reland #40186 and #41220, that were reverted in #41945.

The main modifications from the original PRs are that I predefine the shortcuts and actions maps instead of defining them inline in the build function, and I use a new mapEquals to do a deep comparison so that we don't rebuild modified things if the contents of the map haven't changed.

I also eliminated an operator== and hashCode that were defined on the Actions widget, since widgets shouldn't have those. (it's too bad though: I get an 85% speedup if we leave this in! Too bad it prevents rebuilding of the children...)

Related Issues

#41919
Fixes #40101

Tests

  • Added tests for the collections.dart functions setEquals, listEquals, and the new mapEquals.

Breaking Change

  • No, this is not a breaking change.

@fluttergithubbot fluttergithubbot added f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. c: contributor-productivity Team-specific productivity, code health, technical debt. labels Oct 8, 2019
@codecov
Copy link

codecov bot commented Oct 8, 2019

Codecov Report

Merging #42278 into master will decrease coverage by 1.43%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #42278      +/-   ##
==========================================
- Coverage   60.37%   58.94%   -1.44%     
==========================================
  Files         194      194              
  Lines       18849    18845       -4     
==========================================
- Hits        11381    11109     -272     
- Misses       7468     7736     +268
Flag Coverage Δ
#flutter_tool 58.94% <ø> (-1.44%) ⬇️
Impacted Files Coverage Δ
...ools/lib/src/build_runner/resident_web_runner.dart 33.73% <0%> (-45.19%) ⬇️
packages/flutter_tools/lib/src/run_hot.dart 46.42% <0%> (-23%) ⬇️
...ackages/flutter_tools/lib/src/resident_runner.dart 37.5% <0%> (-19.88%) ⬇️
...ckages/flutter_tools/lib/src/commands/version.dart 78.26% <0%> (-15.22%) ⬇️
...ckages/flutter_tools/lib/src/reporting/events.dart 84.5% <0%> (-11.27%) ⬇️
packages/flutter_tools/lib/src/run_cold.dart 9.75% <0%> (-9.76%) ⬇️
packages/flutter_tools/lib/src/mdns_discovery.dart 77.77% <0%> (-6.35%) ⬇️
packages/flutter_tools/lib/src/context_runner.dart 67.24% <0%> (-1.73%) ⬇️
packages/flutter_tools/lib/src/version.dart 93.23% <0%> (-1.45%) ⬇️
packages/flutter_tools/lib/src/cache.dart 48.47% <0%> (-0.71%) ⬇️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6a63ef7...b06c773. Read the comment docs.

@gspencergoog gspencergoog marked this pull request as ready for review October 9, 2019 23:57
@gspencergoog gspencergoog force-pushed the activate_regression branch 3 times, most recently from 23bb8dc to f184aab Compare October 10, 2019 15:33
Copy link
Contributor

@darrenaustin darrenaustin left a comment

Choose a reason for hiding this comment

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

A lot here, but it LGTM.

@chinmaygarde
Copy link
Member

This commit was identified as a potential cause of a benchmark regression. This is tracked in #42564.

@gspencergoog gspencergoog deleted the activate_regression branch October 15, 2019 21:35
Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Nov 26, 2019
This attempts to reland flutter#40186 and flutter#41220, that were reverted in flutter#41945.

The main modifications from the original PRs are that I predefine the shortcuts and actions maps instead of defining them inline in the build function, and I use a new mapEquals to do a deep comparison so that we don't rebuild modified things if the contents of the map haven't changed.

I also eliminated an operator== and hashCode that were defined on the Actions widget, since widgets shouldn't have those. (it's too bad though: I get an 85% speedup if we leave this in! Too bad it prevents rebuilding of the children...)

Fixes flutter#40101
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

c: contributor-productivity Team-specific productivity, code health, technical debt. f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A11y: Dropdowns have accessibility focus issues

5 participants