Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preserve query parameters on back button navigation #16744

Closed
jorrit-wehelp opened this issue Dec 14, 2018 · 12 comments

Comments

8 participants
@jorrit-wehelp
Copy link

commented Dec 14, 2018

Bug Report

Ionic version:

[x] 4.x

Current behavior:

Query parameters are not preserved when using back button.

Expected behavior:

Preserve any queryParams.

Steps to reproduce:

Two pages, One starts with an queryParam.
Navigate to second page.
Use ion-back-button to go back
The queryParam is gone.

Related code:

─────────────────────────────────────────────────────────────────────────────────────────────────────────
 --- a/angular/src/directives/navigation/stack-utils.ts
 +++ b/angular/src/directives/navigation/stack-utils.ts
──────────────────────────────────────────────────────────────────────────────────────────────────────────
@@ -38,7 +38,7 @@ function setBack(views: RouteView[], view: RouteView) {
 }
 
 export function getUrl(router: Router, activatedRoute: ActivatedRoute) {
   const urlTree = router.createUrlTree(['.'], { relativeTo: activatedRoute });
   const urlTree = router.createUrlTree(['.'], { relativeTo: activatedRoute, queryParams: activatedRoute.snapshot.queryParams });
   return router.serializeUrl(urlTree);
 }

Other information:

Ionic info:

Ionic:

   ionic (Ionic CLI)             : 4.3.0
   Ionic Framework               : @ionic/angular 4.0.0-beta.18
   @angular-devkit/build-angular : 0.10.6
   @angular-devkit/schematics    : 7.0.6
   @angular/cli                  : 7.0.6
   @ionic/angular-toolkit        : 1.2.0

Cordova:

   cordova (Cordova CLI) : 8.1.2 (cordova-lib@8.1.1)
   Cordova Platforms     : android 7.1.4
   Cordova Plugins       : cordova-plugin-ionic-keyboard 2.1.3, cordova-plugin-ionic-webview 2.2.5, (and 12 other plugins)

@ionitron-bot ionitron-bot bot added the triage label Dec 14, 2018

@paulstelzer

This comment has been minimized.

Copy link
Collaborator

commented Dec 14, 2018

Thanks for your issue! This must be a duplicate of #16736 (please confirm or deny)

There is a workaround inside: #16736 (comment)

@paulstelzer paulstelzer added needs: reply and removed triage labels Dec 14, 2018

@jorrit-wehelp

This comment has been minimized.

Copy link
Author

commented Dec 14, 2018

No, it's not a duplicate. This is not regarding the router's ActivatedRoute observables not firing when navigating...
It's about 'storing ' the views in the stack with the URL query parameters, so back button works more like normal history.back(). Only if we keep the query params after a back navigation these could be picked up with the workaround mentioned in the other issue.

@paulstelzer

This comment has been minimized.

Copy link
Collaborator

commented Dec 14, 2018

Alright I understand!

@astec

This comment has been minimized.

Copy link

commented Jan 27, 2019

This is a serious blocker.

/members?commune=abc
=>navForward=> /member?id=123
=>click back button=> /members we've lost commune ID parameter

We've decided to use query URL parameters in PWA for a number of reasons (analytics, filters, etc.) and this bug breaks navigation.

Reproducible in 4.0.0 release.

CC @manucorporat as I see you are working on back button navigation & overlays.

@davidbwire

This comment has been minimized.

Copy link

commented Apr 7, 2019

Was experiencing the same problem on @ionic/angular 4.1.1 but managed to achieve desired behavior by setting defaultHref attribute on all none home to home pages cc @astec

<ion-header>
  <ion-toolbar color="primary">
    <ion-buttons slot="start">
      <ion-back-button defaultHref="home"></ion-back-button>
    </ion-buttons>
    <ion-title>Page Title</ion-title>
  </ion-toolbar>
</ion-header>
@liamdebeasi

This comment has been minimized.

Copy link
Member

commented Apr 10, 2019

Hey there! Can you give this a shot on 4.2.0? There were a few routing fixes that went in with that release.

Thanks!

@ionitron-bot ionitron-bot bot added triage and removed needs: reply labels Apr 13, 2019

@ionitron-bot ionitron-bot bot removed the triage label Apr 15, 2019

@daem0ndev

This comment has been minimized.

Copy link
Contributor

commented Apr 17, 2019

@liamdebeasi I can confirm this is a completely different issue from the fixes we introduced in 4.2. I am working on a PR for this one will update this thread when ready.

Also slightly related but different issue that could potentially be resolved once this issue is resolved: #15428

cc @mhartington

@ionitron-bot ionitron-bot bot added triage and removed needs: reply labels Apr 17, 2019

@daem0ndev

This comment has been minimized.

Copy link
Contributor

commented Apr 17, 2019

Another related issue (not sure if there is an open one for it, but its related here) is that navigating directly to a url with a query string on ionic will trim off the query string. This can be reproduced by simply going to the ionic conference app and directly navigating to http://localhost:4200/app/tabs/schedule?test=ionic and it will trim off the query string before loading the page. Ideally the query string should remain as it would in a regular angular app.

2019-04-16 23 00 56

Edit: interestingly enough, this other issue seems to only manifest when using tabs. If you go to the tutorial route, which does not use /app/tabs/ that one will keep the query string

@lucassch

This comment has been minimized.

Copy link

commented Apr 30, 2019

still happens in @ionic/angular 4.3.3

@sdeprada

This comment has been minimized.

Copy link

commented May 1, 2019

I think this behaviour begins with @ionic/angular 4.2.0 version
With "@ionic/angular": "^4.1.2" version, query parameters are preserved on back button

@liamdebeasi liamdebeasi added this to Backlog 🤖 in Ionic Core via automation May 17, 2019

@ionitron-bot ionitron-bot bot removed the triage label May 17, 2019

@liamdebeasi liamdebeasi moved this from Backlog 🤖 to In progress 🤺 in Ionic Core May 17, 2019

@liamdebeasi liamdebeasi moved this from In progress 🤺 to Needs review 🤔 in Ionic Core May 17, 2019

Ionic Core automation moved this from Needs review 🤔 to Done 🎉 May 20, 2019

@liamdebeasi

This comment has been minimized.

Copy link
Member

commented May 20, 2019

Thanks for the issue! This has been resolved via #18298 and will be in the next release of Ionic.

In the meantime, I have created a nightly build if people would like to test it out and provide feedback. (npm i @ionic/angular@dev).

Thanks!

brandyscarney added a commit that referenced this issue May 22, 2019

merge release-4.4.1
* docs(process): update release process

* docs(fab-list): update the activated description (#18026)

* docs(breaking): add ionDrag event arguments change (#17989)

* docs(slides): add swiper prefix in animation usage (#18073)

* feat(searchbar): add disabled property (#17935)

closes #17921

* fix(reorder-group): remove required parameter for the complete method (#18084)

also updates documentation surrounding the reorder & infinite scroll

fixes #16302

* docs(components): update method and parameter descriptions (#18075)

* fix(datetime): default to current date when value is null (#18105)

fixes #18099

* docs(toolbar): fix end slot documentation (#18092)

* fix(item): use the global activated background for md ripple color (#16752)

fixes #16585

* fix(textarea): reposition textarea when keybard appears (#18098)

fixes #17847

* fix(button): apply round property to button sizes in iOS (#18125)

fixes #18108

* fix(): add prefixed transform for older versions of chrome (#18128)

fixes #17729

* fix(segment): decrease icon size on ios and stretch segment buttons to fill height (#17751)

fixes #17069

* fix(): sanitize components using innerHTML (#18083)

fixes #18065

* Release 4.3.1 (#18152) (#18154)

* fix(angular): support replaceUrl with angular <7.2 (#18106)

* fix(angular): support replaceUrl with angular <7.2

* run linter

* fix(): sanitize components using innerHTML (#18146)

* 4.3.1 (#18150)

* doc(loading): remove mention of undefined "content" property (#18126)

* feat(img): add ionImgWillLoad event and emit ionImgDidLoad when image is loaded (#18159)

- Adds `ionImgWillLoad` event that emits when the img src is set
- Moves the `ionImgDidLoad` event emit so that it happens when the image actually finishes loading

fixes #17652 closes #18161

* fix(toast): allow button-color CSS variable to be overridden (#18133)

fixes #18127

* fix(label): use primary color on focus for md input labels (#18183)

fixes #15602

* feat(item-sliding): add open method (#17964)

resolves #17899

* feat(menu-button): add css variables for padding (#18188)

fixes #18187

* feat(card): add button functionality (#17997)

closes #17773

* feat(textarea): add option to expand textarea as value changes (#16916)

* feat(textarea): add autoGrow - set height to scrollHeight

* change 1px to inherit, remove additional 4px

* feat(refresher): add pullFactor property to control speed (#16697)

closes #15425

* fix(input): clear on edit from inside native input (#17115)

fixes #17055

* test(angular): increase timeout for tab switch (#18221)

* 4.4.0

* Release 4.3.1 (#18152)

* fix(angular): support replaceUrl with angular <7.2 (#18106)

* fix(angular): support replaceUrl with angular <7.2

* run linter

* fix(): sanitize components using innerHTML (#18146)

* 4.3.1 (#18150)

* merge release-4.4.0

* docs(process): update release process

* docs(fab-list): update the activated description (#18026)

* docs(breaking): add ionDrag event arguments change (#17989)

* docs(slides): add swiper prefix in animation usage (#18073)

* feat(searchbar): add disabled property (#17935)

closes #17921

* fix(reorder-group): remove required parameter for the complete method (#18084)

also updates documentation surrounding the reorder & infinite scroll

fixes #16302

* docs(components): update method and parameter descriptions (#18075)

* fix(datetime): default to current date when value is null (#18105)

fixes #18099

* docs(toolbar): fix end slot documentation (#18092)

* fix(item): use the global activated background for md ripple color (#16752)

fixes #16585

* fix(textarea): reposition textarea when keybard appears (#18098)

fixes #17847

* fix(button): apply round property to button sizes in iOS (#18125)

fixes #18108

* fix(): add prefixed transform for older versions of chrome (#18128)

fixes #17729

* fix(segment): decrease icon size on ios and stretch segment buttons to fill height (#17751)

fixes #17069

* fix(): sanitize components using innerHTML (#18083)

fixes #18065

* Release 4.3.1 (#18152) (#18154)

* fix(angular): support replaceUrl with angular <7.2 (#18106)

* fix(angular): support replaceUrl with angular <7.2

* run linter

* fix(): sanitize components using innerHTML (#18146)

* 4.3.1 (#18150)

* doc(loading): remove mention of undefined "content" property (#18126)

* feat(img): add ionImgWillLoad event and emit ionImgDidLoad when image is loaded (#18159)

- Adds `ionImgWillLoad` event that emits when the img src is set
- Moves the `ionImgDidLoad` event emit so that it happens when the image actually finishes loading

fixes #17652 closes #18161

* fix(toast): allow button-color CSS variable to be overridden (#18133)

fixes #18127

* fix(label): use primary color on focus for md input labels (#18183)

fixes #15602

* feat(item-sliding): add open method (#17964)

resolves #17899

* feat(menu-button): add css variables for padding (#18188)

fixes #18187

* feat(card): add button functionality (#17997)

closes #17773

* feat(textarea): add option to expand textarea as value changes (#16916)

* feat(textarea): add autoGrow - set height to scrollHeight

* change 1px to inherit, remove additional 4px

* feat(refresher): add pullFactor property to control speed (#16697)

closes #15425

* fix(input): clear on edit from inside native input (#17115)

fixes #17055

* test(angular): increase timeout for tab switch (#18221)

* 4.4.0

* fix other merge conflict

* chore(): resolve conflicts from older hotifx

* Release 4.3.1 (#18152)

* fix(angular): support replaceUrl with angular <7.2 (#18106)

* fix(angular): support replaceUrl with angular <7.2

* run linter

* fix(): sanitize components using innerHTML (#18146)

* 4.3.1 (#18150)

* merge release-4.4.0

* docs(process): update release process

* docs(fab-list): update the activated description (#18026)

* docs(breaking): add ionDrag event arguments change (#17989)

* docs(slides): add swiper prefix in animation usage (#18073)

* feat(searchbar): add disabled property (#17935)

closes #17921

* fix(reorder-group): remove required parameter for the complete method (#18084)

also updates documentation surrounding the reorder & infinite scroll

fixes #16302

* docs(components): update method and parameter descriptions (#18075)

* fix(datetime): default to current date when value is null (#18105)

fixes #18099

* docs(toolbar): fix end slot documentation (#18092)

* fix(item): use the global activated background for md ripple color (#16752)

fixes #16585

* fix(textarea): reposition textarea when keybard appears (#18098)

fixes #17847

* fix(button): apply round property to button sizes in iOS (#18125)

fixes #18108

* fix(): add prefixed transform for older versions of chrome (#18128)

fixes #17729

* fix(segment): decrease icon size on ios and stretch segment buttons to fill height (#17751)

fixes #17069

* fix(): sanitize components using innerHTML (#18083)

fixes #18065

* Release 4.3.1 (#18152) (#18154)

* fix(angular): support replaceUrl with angular <7.2 (#18106)

* fix(angular): support replaceUrl with angular <7.2

* run linter

* fix(): sanitize components using innerHTML (#18146)

* 4.3.1 (#18150)

* doc(loading): remove mention of undefined "content" property (#18126)

* feat(img): add ionImgWillLoad event and emit ionImgDidLoad when image is loaded (#18159)

- Adds `ionImgWillLoad` event that emits when the img src is set
- Moves the `ionImgDidLoad` event emit so that it happens when the image actually finishes loading

fixes #17652 closes #18161

* fix(toast): allow button-color CSS variable to be overridden (#18133)

fixes #18127

* fix(label): use primary color on focus for md input labels (#18183)

fixes #15602

* feat(item-sliding): add open method (#17964)

resolves #17899

* feat(menu-button): add css variables for padding (#18188)

fixes #18187

* feat(card): add button functionality (#17997)

closes #17773

* feat(textarea): add option to expand textarea as value changes (#16916)

* feat(textarea): add autoGrow - set height to scrollHeight

* change 1px to inherit, remove additional 4px

* feat(refresher): add pullFactor property to control speed (#16697)

closes #15425

* fix(input): clear on edit from inside native input (#17115)

fixes #17055

* test(angular): increase timeout for tab switch (#18221)

* 4.4.0

* chore(): resolve merge conflicts from older hotfix

* docs(process): update release process

* docs(fab-list): update the activated description (#18026)

* docs(breaking): add ionDrag event arguments change (#17989)

* docs(slides): add swiper prefix in animation usage (#18073)

* feat(searchbar): add disabled property (#17935)

closes #17921

* fix(reorder-group): remove required parameter for the complete method (#18084)

also updates documentation surrounding the reorder & infinite scroll

fixes #16302

* docs(components): update method and parameter descriptions (#18075)

* fix(datetime): default to current date when value is null (#18105)

fixes #18099

* docs(toolbar): fix end slot documentation (#18092)

* fix(item): use the global activated background for md ripple color (#16752)

fixes #16585

* fix(textarea): reposition textarea when keybard appears (#18098)

fixes #17847

* fix(button): apply round property to button sizes in iOS (#18125)

fixes #18108

* fix(): add prefixed transform for older versions of chrome (#18128)

fixes #17729

* fix(segment): decrease icon size on ios and stretch segment buttons to fill height (#17751)

fixes #17069

* fix(): sanitize components using innerHTML (#18083)

fixes #18065

* Release 4.3.1 (#18152) (#18154)

* fix(angular): support replaceUrl with angular <7.2 (#18106)

* fix(angular): support replaceUrl with angular <7.2

* run linter

* fix(): sanitize components using innerHTML (#18146)

* 4.3.1 (#18150)

* doc(loading): remove mention of undefined "content" property (#18126)

* feat(img): add ionImgWillLoad event and emit ionImgDidLoad when image is loaded (#18159)

- Adds `ionImgWillLoad` event that emits when the img src is set
- Moves the `ionImgDidLoad` event emit so that it happens when the image actually finishes loading

fixes #17652 closes #18161

* fix(toast): allow button-color CSS variable to be overridden (#18133)

fixes #18127

* fix(label): use primary color on focus for md input labels (#18183)

fixes #15602

* feat(item-sliding): add open method (#17964)

resolves #17899

* feat(menu-button): add css variables for padding (#18188)

fixes #18187

* feat(card): add button functionality (#17997)

closes #17773

* feat(textarea): add option to expand textarea as value changes (#16916)

* feat(textarea): add autoGrow - set height to scrollHeight

* change 1px to inherit, remove additional 4px

* feat(refresher): add pullFactor property to control speed (#16697)

closes #15425

* fix(input): clear on edit from inside native input (#17115)

fixes #17055

* test(angular): increase timeout for tab switch (#18221)

* fix other merge conflict

* fix(react): Support for adding css classes via `className` in Ionic React components (#18231)

* fix(react): adding classname to react props

* fix(react): updating rtl to latest to fix ts error

* fix(react): changes to support className

* fix(loading): allow html content (#18242)

fixes #18135

* fix(icon): remove stroke and move fill to host element (#18241)

This removes the weird border around custom SVGs used in an ion-icon.

fixes #16483

* bug(security): allow name and slot attributes when sanitizing (#18246)

* allow name attribute

* also add slot

* fix(input): keep entire input in view when scrolling with keyboard open (#18253)

fixes #17457

* fix(tab-button): apply background-focused when tabbing into tab button (#17502)

fixes #17042

* test(theming): update theming tests

* fix(react): defaultHref fixes (#18278)

* fix(react): making children prop optional on overlay components

* fix(react): passing in defaultHref so it can be used if there is no prev view

* fix(react): making children prop optional on overlay components (#18243)

* fix(buttons): use theme/color toolbar colors for buttons (#18191)

- Updates the iOS buttons in a toolbar to use the proper global theming variables
- Updates the iOS segment to use the correct background variable when checked
- Updates the iOS back button and menu button to use the proper color in a toolbar
- Updates the iOS buttons in a toolbar w/ color to use the proper contrast colors (background, borders, text, hover, focus), mostly solid and outline buttons were affected
- Updates the CSS that applies the global toolbar variables so that it won't affect toolbars w/ a color

fixes #18184, fixes #17840

* fix(overlay): hide scrollbars on non-scrollable content (#16767)

fixes #14178

* fix(slides): disable swiper touch preventDefault (#16728)

* fix(slides): disable swiper touch preventDefault

* fix(slides): update Swiper types

* add screenshots to test

* add screenshot descriptions

* fix(toolbar): update md toolbar button spacing and padding to match spec (#17537)

- Removes the padding from the main toolbar and individually style the components inside of it
- Adds a `has-icon-only` class to button, this is used to switch between `unbounded` and `bounded` ripples on buttons in a toolbar. If the button is clear and only has an icon, we use the unbounded "circular" ripple effect, otherwise still use the bounded one. This matches the MD spec, without making the other buttons look off.
- Using the class above, style the button differently to match the MD spec
- Updates the back button and menu button to use the proper size / icon size
- Removes the opacity on an activated back button, it should use the ripple for activated
- Moves the margin to the slots in a toolbar by grabbing the "first" and "last" slot and applying a class to them
- Makes the segment in a toolbar use the min height from the toolbar
- Updates the back button so that it matches the MD spec
- Updates the header box shadow to use the old v3 datauri 

fixes #16950 fixes #14444

* fix(tabs): initialize select in the willLoad before the select call is made (#18300)

#17957

* fix(angular): preserve queryParams and fragment when going back (#18298)

fixes #16744

* fix(angular): ensure active page is not removed from change detection (#18299)

fixes #18293

* docs(toast-controller): fix description typo  (#18312)

Fix typo in component description text.

* test(components): add rtl tests and remove skips (#18319)

references #17012

* fix(css): update rtl function to prepend selectors with host-context properly (#18315)

references #17012

* chore(): remove debug and log statements (#18245)

fixes #18190

* docs(datetime): add note about timezones, fix typo (#18289)

* fix(range): update border-radius on range pin for rtl (#18321)

references #17012

* fix(angular): preserve special characters encoding when going back (#18323)

* fix(rtl): updates searchbar, fab and toggle icon positioning in rtl (#18325)

- fixes tab badge (in Chrome)
- fixes searchbar buttons
- fixes fab positioning
- fixes toggle

references #17012

* fix(segment): update segment border for rtl (#18326)

references #17012

* fix(datetime): update label direction in rtl (#18340)

* fix(picker): update the column positions in rtl (#18339)

references #17012

* fix(button): only apply has-icon-only if icon has the slot for icon-only (#18343)

fixes #18329

* tests(): add missing test statements (#18346)

* 4.4.1

* remove react changelog
@ionitron-bot

This comment has been minimized.

Copy link

commented Jun 19, 2019

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Jun 19, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.