Skip to content

Conversation

@Hixie
Copy link
Contributor

@Hixie Hixie commented May 15, 2021

Instead of having two ways to update the engine about the current route, this moves everything to one API, and separately selects the single-entry history mode if you create a Navigator that reports routes.

Also fixes #82574

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 feature I am adding, or Hixie said the PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@flutter-dashboard flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label May 15, 2021
@google-cla google-cla bot added the cla: yes label May 15, 2021
@Hixie Hixie force-pushed the nav branch 2 times, most recently from 1b18b00 to b068dfc Compare May 17, 2021 22:22
@Hixie Hixie force-pushed the nav branch 4 times, most recently from 6b430b7 to bf8184f Compare May 18, 2021 02:13
Copy link
Contributor

@chunhtai chunhtai left a comment

Choose a reason for hiding this comment

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

I think we can call selectMultiEntryHistory in PlatformRouteInformationProvider.routerReportsNewRouteInformation?

@Hixie
Copy link
Contributor Author

Hixie commented May 18, 2021

Ok, added.

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

@goderbauer
Copy link
Member

... but looks like some tests are failing...

@Hixie Hixie force-pushed the nav branch 2 times, most recently from a5392dc to 9cf4b45 Compare May 20, 2021 04:13
@Hixie Hixie marked this pull request as ready for review May 20, 2021 06:08
@Hixie
Copy link
Contributor Author

Hixie commented May 20, 2021

Blocked on flutter/engine#26164 rolling into the framework.

Instead of having two ways to update the engine about the current route, this moves everything to one API, and separately selects the single-entry history mode if you create a Navigator that reports routes.

Also fixes flutter#82574
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.

API docs for SystemNavigator.routeInformationUpdated and SystemNavigator.routeUpdated

4 participants