Skip to content

Conversation

@gspencergoog
Copy link
Contributor

Description

This removes Shorcuts.of and Shortctus.maybeOf because they're not especially useful, since the only thing you can really set on a ShortcutManager is the shortcuts, and the Shortcuts widget that you give it to manages those, so if it rebuilds, it overwrites what you set.

Also, adds a Shortcuts.manager constructor and removes the manager argument to the Shortcuts widget.

Removing these will also eliminate an InheritedWidget for each Shortcuts widget, improving memory usage.

Tests

  • Update tests, added tests for Shortcuts.manager.

@flutter-dashboard flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label May 19, 2022
@gspencergoog gspencergoog changed the title Deprecating Shorcuts.of and Shortctus.maybeOf Removing Shorcuts.of and Shortctus.maybeOf May 19, 2022
@gspencergoog gspencergoog force-pushed the remove_shortcut_manager_of branch from 682ba48 to a20076a Compare May 19, 2022 22:36
@gspencergoog gspencergoog requested a review from goderbauer May 19, 2022 22:36
Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM if this doesn't break anything.

Copy link
Member

Choose a reason for hiding this comment

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

this field will stay null if the unnamed constructor is used, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh! yes. Duh.

@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

CaseyHillers added a commit that referenced this pull request May 26, 2022
dkwingsmt added a commit to dkwingsmt/flutter that referenced this pull request Jun 1, 2022
camsim99 pushed a commit to camsim99/flutter that referenced this pull request Aug 10, 2022
This removes Shorcuts.of and Shortctus.maybeOf because they're not especially useful, since the only thing you can really set on a ShortcutManager is the shortcuts, and the Shortcuts widget that you give it to manages those, so if it rebuilds, it overwrites what you set.

Also, adds a Shortcuts.manager constructor and removes the manager argument to the Shortcuts widget.

Removing these will also eliminate an InheritedWidget for each Shortcuts widget, improving memory usage.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants