Skip to content

Access current location using uri.path to support deep links - #6474

Merged
auto-submit[bot] merged 6 commits into
flutter:mainfrom
AhmedLSayed9:update_location_access
Apr 11, 2024
Merged

Access current location using uri.path to support deep links#6474
auto-submit[bot] merged 6 commits into
flutter:mainfrom
AhmedLSayed9:update_location_access

Conversation

@AhmedLSayed9

@AhmedLSayed9AhmedLSayed9 commented Apr 6, 2024

Copy link
Copy Markdown
Contributor

This PR updates examples to use uri.path instead of uri.toString() for accessing the current location.

While the examples don't use deep linking, promoting the usage of uri.toString() in the examples doesn't seem to be a good idea as it can lead to issues when it's used with deep links (It'll include host and scheme).

Pre-launch Checklist

chunhtai
chunhtai reviewed Apr 8, 2024
View reviewed changes

@chunhtaichunhtai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good call. This change looks good to me, can you also bump package version and change log?

@chunhtai

chunhtai commented Apr 8, 2024

Copy link
Copy Markdown
Contributor

Some of the tests are failing. can you fix them as well?

@AhmedLSayed9
AhmedLSayed9force-pushed the update_location_access branch from db04931 to 05dfb08CompareApril 8, 2024 17:41
@AhmedLSayed9

AhmedLSayed9 commented Apr 8, 2024•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Some of the tests are failing. can you fix them as well?

When passing a query parameter, i.e doing: router.go('/?p=123'):

  • GoRouterState.of(context).uri.path result in: /
  • GoRouterState.of(context).matchedLocation result in: /
  • GoRouterState.of(context).uri result in: /?p=123

So, accessing current location using uri.path will not include query params. Is that ok?

If that's fine, I can fix the tests by checking for GoRouterState.of(context).uri.queryParameters individually.

@AhmedLSayed9

AhmedLSayed9 commented Apr 10, 2024

Copy link
Copy Markdown
ContributorAuthor

@chunhtai
I'm waiting for your answer before going further with fixing the tests :)

@chunhtai

chunhtai commented Apr 10, 2024•
edited
Loading

Copy link
Copy Markdown
Contributor

I think you can just change the expect to ignore query parameter since now the example does not print query parameter onto the page.

If that's fine, I can fix the tests by checking for GoRouterState.of(context).uri.queryParameters individually.

I am fine with it

@AhmedLSayed9
AhmedLSayed9force-pushed the update_location_access branch from adfb7ae to ab6f793CompareApril 10, 2024 20:23
@AhmedLSayed9

AhmedLSayed9 commented Apr 10, 2024

Copy link
Copy Markdown
ContributorAuthor

@chunhtai Can you check now?

chunhtai
chunhtai approved these changes Apr 10, 2024
View reviewed changes

@chunhtaichunhtai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

hannah-hyj
hannah-hyj approved these changes Apr 10, 2024
View reviewed changes

@hannah-hyjhannah-hyj left a comment

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.

LGTM

@chunhtai

chunhtai commented Apr 10, 2024

Copy link
Copy Markdown
Contributor

looks like there are still some ci error

@AhmedLSayed9

AhmedLSayed9 commented Apr 11, 2024

Copy link
Copy Markdown
ContributorAuthor

@chunhtai
There was a failing test for all_types example, I've fixed it.

@hannah-hyjhannah-hyj added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 11, 2024
@auto-submit
auto-submitBot merged commit 488da36 into flutter:mainApr 11, 2024
@engine-flutter-autorollengine-flutter-autoroll mentioned this pull request Apr 12, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 12, 2024
@engine-flutter-autoroll
488da36Access current location using uri.path to support deep links (f…
0c139c1
…lutter/packages#6474)
@engine-flutter-autorollengine-flutter-autoroll mentioned this pull request Apr 12, 2024
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 12, 2024
@engine-flutter-autoroll
488da36Access current location using uri.path to support deep links (f…
d30050e
…lutter/packages#6474)
auto-submitBot pushed a commit to flutter/flutter that referenced this pull request Apr 12, 2024
@engine-flutter-autoroll
Roll Packages frome98839ato78f684c(6 revisions) (#146691)
699749e
flutter/packages@e98839a...78f684c
2024-04-12 engine-flutter-autoroll@skia.org Roll Flutter from 557fbf5 to 53cba24 (11 revisions) (flutter/packages#6509)
2024-04-11 stuartmorgan@google.com [file_selector] Remove OCMock from iOS implementation (flutter/packages#6503)
2024-04-11 ahmedelsaayid@gmail.com Access current location using uri.path to support deep links (flutter/packages#6474)
2024-04-11 engine-flutter-autoroll@skia.org Roll Flutter from 97cd47a to 557fbf5 (22 revisions) (flutter/packages#6502)
2024-04-11 louisehsu@google.com [packages] Set parallelizable to NO to reduce test flakiness in packages project tests (flutter/packages#6471)
2024-04-11 vbuberen@users.noreply.github.com Update multiple packages to depend on versions with iOS privacy manifest included (flutter/packages#6355)
If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human
is aware of the problem.
To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose
To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622
Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
gilnobrega pushed a commit to gilnobrega/flutter that referenced this pull request Apr 22, 2024
@engine-flutter-autoroll@gilnobrega
Roll Packages frome98839ato78f684c(6 revisions) (flutter#146691)
2e4e9d8
flutter/packages@e98839a...78f684c
2024-04-12 engine-flutter-autoroll@skia.org Roll Flutter from 557fbf5 to 53cba24 (11 revisions) (flutter/packages#6509)
2024-04-11 stuartmorgan@google.com [file_selector] Remove OCMock from iOS implementation (flutter/packages#6503)
2024-04-11 ahmedelsaayid@gmail.com Access current location using uri.path to support deep links (flutter/packages#6474)
2024-04-11 engine-flutter-autoroll@skia.org Roll Flutter from 97cd47a to 557fbf5 (22 revisions) (flutter/packages#6502)
2024-04-11 louisehsu@google.com [packages] Set parallelizable to NO to reduce test flakiness in packages project tests (flutter/packages#6471)
2024-04-11 vbuberen@users.noreply.github.com Update multiple packages to depend on versions with iOS privacy manifest included (flutter/packages#6355)
If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human
is aware of the problem.
To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose
To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622
Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
TecHaxter pushed a commit to TecHaxter/flutter_packages that referenced this pull request May 22, 2024
@AhmedLSayed9@TecHaxter
Access current location using uri.path to support deep links (flutter…
71ab378
…#6474)
This PR updates examples to use `uri.path` instead of `uri.toString()` for accessing the current location.
While the examples don't use deep linking, promoting the usage of `uri.toString()` in the examples doesn't seem to be a good idea as it can lead to issues when it's used with deep links (It'll include host and scheme).
creatorpiyush pushed a commit to creatorpiyush/packages that referenced this pull request Jun 10, 2026
@AhmedLSayed9
Access current location using uri.path to support deep links (flutter…
7765cea
…#6474)
This PR updates examples to use `uri.path` instead of `uri.toString()` for accessing the current location.
While the examples don't use deep linking, promoting the usage of `uri.toString()` in the examples doesn't seem to be a good idea as it can lead to issues when it's used with deep links (It'll include host and scheme).
bisor0627 pushed a commit to bisor0627/packages that referenced this pull request Jun 19, 2026
@AhmedLSayed9
Access current location using uri.path to support deep links (flutter…
5e438e0
…#6474)
This PR updates examples to use `uri.path` instead of `uri.toString()` for accessing the current location.
While the examples don't use deep linking, promoting the usage of `uri.toString()` in the examples doesn't seem to be a good idea as it can lead to issues when it's used with deep links (It'll include host and scheme).
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chunhtaichunhtaichunhtai approved these changes
@hannah-hyjhannah-hyjhannah-hyj approved these changes

Assignees

No one assigned

Labels

autosubmitMerge PR when tree becomes green via auto submit Appp: go_router_builderp: go_router

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@AhmedLSayed9@chunhtai@hannah-hyj