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

bug: Ionic React navigate with push from useIonRouter ignores direction when used as "replace" #24260

Closed
4 of 6 tasks
babycourageous opened this issue Nov 23, 2021 · 10 comments · Fixed by #28671
Closed
4 of 6 tasks
Labels
package: react @ionic/react package type: bug a confirmed bug report

Comments

@babycourageous
Copy link
Contributor

Prerequisites

Ionic Framework Version

  • v4.x
  • v5.x
  • v6.x

Current Behavior

using Ionic React and useIonRouter the direction is not respected if the route action is "replace". It always goes "forward"
If the action is "push" (the default) all directions respected.

Expected Behavior

Direction of the router should behave based on function call - no matter the route action.

Steps to Reproduce

In the repo, fire up the app and you should go to the EXAMPLE menu item. Here there are three links to detail pages.
Detail 1 has a button to redirect using "replace" with direction "none" - this will redirect but animate using "forward"
Detail 2 has a button to redirect using default behavior ("push") with direction "none" - this will redirect and have no page transition animation.

Code Reproduction URL

https://github.com/babycourageous/ionic-use-ion-router-direction-bug

Ionic Info

Ionic:

Ionic CLI : 6.17.1 (/Users/renedellefont/.fnm/node-versions/v14.17.6/installation/lib/node_modules/@ionic/cli)
Ionic Framework : @ionic/react 5.8.4

Capacitor:

Capacitor CLI : 3.2.5
@capacitor/android : not installed
@capacitor/core : 3.2.5
@capacitor/ios : not installed

Utility:

cordova-res : not installed globally
native-run : 1.5.0

System:

NodeJS : v14.17.6 (/Users/renedellefont/.fnm/node-versions/v14.17.6/installation/bin/node)
npm : 6.14.15
OS : macOS Big Sur

Additional Information

Just let me know if there's any more info you need!

@ionitron-bot ionitron-bot bot added the triage label Nov 23, 2021
@sean-perkins sean-perkins added package: react @ionic/react package type: bug a confirmed bug report labels Nov 23, 2021
@gregorgretz
Copy link

i stumbled across the same problem and can confirm the described behavior

@fr3fou
Copy link

fr3fou commented Dec 21, 2021

any updates? I've been dealing with the same thing

@gregorgretz
Copy link

in my use case I just wanted to use 'none' as routerDirection to remove the page animation. My workaround was to pass the animationBuilder property and use an empty animation like that:

router.push(n.href, 'none', 'replace', {}, createAnimation());

maybe this will help someone

@ankrut
Copy link

ankrut commented Mar 25, 2022

Same issue with v6.x. I also noticed that it works only if there is no history yet. For example, if the first push is a back with replace then it will work properly with a back animation. But then all following push calls (with replace) will result in a back transition, regardless if the routerDirection is back or forward. Analog behavior if the first push is a forward.

So, a push with a replace seems to ignore the routerDirection unless it is the first call with no history yet.

I found the following workaround

const router = useIonRouter()

// navigation back
router.routeInfo.routeDirection = 'back'
router.push('some/path', 'back', 'replace')

// navigation forward
router.routeInfo.routeDirection = 'forward'
router.push('some/path', 'forward', 'replace')

@aeharding
Copy link
Contributor

in my use case I just wanted to use 'none' as routerDirection to remove the page animation. My workaround was to pass the animationBuilder property and use an empty animation like that:

router.push(n.href, 'none', 'replace', {}, createAnimation());

maybe this will help someone

This works, the only drawback is that the swipe back animation is broken.

@TylerAHolden
Copy link

I do not know why it works but I had to do a mix of both of these solutions....

ionRouter.routeInfo.routeDirection = 'none';
ionRouter.push(url, 'none', 'replace', {}, () => createAnimation());

@aeharding
Copy link
Contributor

@TylerAHolden Unfortunately that doesn't solve it for me, the swipe back animation still doesn't work (due to forcing it off from createAnimation() call)

@TylerAHolden
Copy link

@TylerAHolden Unfortunately that doesn't solve it for me, the swipe back animation still doesn't work (due to forcing it off from createAnimation() call)

Ah yes the swipe back still doesn't work.

@liamdebeasi
Copy link
Member

Hi everyone,

Here is a dev build with a proposed fix if anyone is interested in testing:

npm install @ionic/react@7.6.1-dev.11702046065.12315cc7 @ionic/react-router@7.6.1-dev.11702046065.12315cc7

github-merge-queue bot pushed a commit that referenced this issue Dec 11, 2023
…28671)

Issue number: resolves #24260

---------

<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->

<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->

## What is the current behavior?

When replacing a route (`router.push(newRoute, 'none', 'replace')`) the
`RouterDirection` from the route being replaced is being used (if it
exists) instead of the new one the user specifies.

## What is the new behavior?

User-specified `RouteDirection` is used, if it exists. If it doesn't it
falls back to the `RouteDirection` of the route being replaced.

## Does this introduce a breaking change?

- [ ] Yes
- [X] No

<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->


## Other information

Please see the following comment for why I think the current behavior is
incorrect, and why this change is needed:
#24260 (comment)

---------

Co-authored-by: Liam DeBeasi <liamdebeasi@users.noreply.github.com>
Copy link

ionitron-bot bot commented Jan 10, 2024

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 Jan 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: react @ionic/react package type: bug a confirmed bug report
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants