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

ionViewCanLeave lifecycle hook on Tabs page - error with ion-segments #9392

Closed
kelvindart opened this issue Nov 28, 2016 · 15 comments
Closed
Assignees
Milestone

Comments

@kelvindart
Copy link
Contributor

kelvindart commented Nov 28, 2016

Ionic version: (check one with "x")
[ ] 1.x
[x] 2.x

I'm submitting a ... (check one with "x")
[x] bug report
[ ] feature request
[ ] support request => Please do not submit support requests here, use one of these channels: https://forum.ionicframework.com/ or http://ionicworldwide.herokuapp.com/

Current behavior:
I believe this to be a bug, although you may correct me if I am wrong: using an ionViewCanLeave() lifecycle hook on a tabs page which implements ion-segments seems to cause the ion-segment-buttons to stop operating after rejecting the promise.

E.g. I have a tab button hooked up to a logout button with an ionSelect implemented. This ionSelect fires my logout logic (which pops the page). After tapping "No" to confirm if I want to log out, the ion-segment seems to be inoperable.

This seems to be the case when the hook is implemented on the main tabs TypeScript file.

Expected behavior:
For the ion-segment to continue working, after rejecting the promise returned in ionViewCanLeave.

Steps to reproduce:

  1. Clone the following repository: https://github.com/kelvindart/ionic2-tabs-lifecycle.
  2. mkdir www so Ionic/Cordova recognises it as a Cordova project.
  3. Run npm install.
  4. Run ionic serve.
  5. Tap "Go to Tabs".
  6. Tap "Log out".
  7. Tap "No".
  8. Attempt to tap each segment, observe the [ngSwitch] does not operate.

Related code:

  ionViewCanLeave() {
    return new Promise((resolve, reject) => {
      let alert = this.alertCtrl.create({
        title: 'Log out',
        subTitle: 'Are you sure you want to log out?',
        buttons: [
          {
            text: 'No',
            role: 'cancel',
            handler: () => {
              reject();
            }
          },
          {
            text: 'Yes',
            handler: () => {
              alert.dismiss().then(() => {
                resolve();
              });
            }
          }
        ]
      });

      alert.present();
    });

Other information:
N/A

Ionic info: (run ionic info from a terminal/cmd prompt and paste output below):

Cordova CLI: 6.4.0 
Ionic Framework Version: 2.0.0-rc.3
Ionic CLI Version: 2.1.12
Ionic App Lib Version: 2.1.7
Ionic App Scripts Version: 0.0.45
ios-deploy version: Not installed
ios-sim version: 5.0.8 
OS: macOS Sierra
Node Version: v7.2.0
Xcode version: Xcode 8.1 Build version 8B62
@kelvindart kelvindart changed the title ionViewCanLeave lifecycle hook on Tabs page error ionViewCanLeave lifecycle hook on Tabs page - error with ion-segments Nov 28, 2016
@manucorporat
Copy link
Contributor

@kelvindart can you quickly provide a plunker that reproduces the issue?
http://plnkr.co/edit/GJte2b?p=preview

@manucorporat manucorporat self-assigned this Nov 30, 2016
@kelvindart
Copy link
Contributor Author

kelvindart commented Nov 30, 2016

Hi @manucorporat, Plunker created here: http://plnkr.co/edit/QB4vcaQuIhKLYXSgyGvl?p=preview.

Steps to replicate:

  1. Load above Plunker.
  2. Tap "Go to Tabs"
  3. Observe the segment switches at the top (currently you can switch between "Camera" and "Bookmark").
  4. Tap the log out tab (bottom-right).
  5. Tap "No".
  6. Repeat step (3) and observe no switch.

Let me know if you require any more info! Thanks.

@manucorporat
Copy link
Contributor

@kelvindart thanks!

@kelvindart
Copy link
Contributor Author

Hey @manucorporat - apologies for that. The link has been updated (http://plnkr.co/edit/QB4vcaQuIhKLYXSgyGvl?p=preview for convenience sake).

@kelvindart
Copy link
Contributor Author

Hey @manucorporat - just curious, did you manage to replicate this? I understand you're busy, so just a question in passing.

@manucorporat
Copy link
Contributor

@kelvindart yes! I can reproduce it, looking into it! Thanks again for the plunker!

@kelvindart
Copy link
Contributor Author

Legend.

manucorporat added a commit to manucorporat/ionic that referenced this issue Dec 6, 2016
The current selected tab should NOT be deselected (i.e. detached from change detection) if the selected tab does not have a root
ie. a tab that acts as a button to open a modal, logout etc.

Lifecycle events should not be dispatched either. Right now we are dispatching willLeave/willEnter always (this is a bug).

fixes ionic-team#9392
@manucorporat
Copy link
Contributor

manucorporat commented Dec 6, 2016

@kelvindart I have one fix: #9512
but I am not sure it will be included in the next release, it is a big change, and we already started testing.

@manucorporat manucorporat added this to the 2.0.0-rc.5 milestone Dec 6, 2016
@kelvindart
Copy link
Contributor Author

Hi @manucorporat - that's awesome! Thank you very much :)

@kelvindart
Copy link
Contributor Author

Ah I see the problem - it's because clicking the "Logout" tab was then setting the selected tab to that tab, although we're never actually selecting/activating it?

@manucorporat
Copy link
Contributor

@kelvindart yeah kind of, it was detaching the current tab from ng2 change detection

@kelvindart
Copy link
Contributor Author

Hey Manu, I noticed this had the rc.5 tag removed. Will it not be included in the next release? Is there anything I can do to assist?

mhartington pushed a commit that referenced this issue Jan 6, 2017
The current selected tab should NOT be deselected (i.e. detached from change detection) if the selected tab does not have a root
ie. a tab that acts as a button to open a modal, logout etc.

Lifecycle events should not be dispatched either. Right now we are dispatching willLeave/willEnter always (this is a bug).

Closes #9392. Closes #9811. Closes #9392.
@brandyscarney
Copy link
Member

This should be fixed in the next release. To test it sooner, please install the latest nightly:

npm install --save ionic-angular@nightly

Let us know if you see any issues. Thanks! 😄

@kelvindart
Copy link
Contributor Author

Hey @brandyscarney - that's awesome, thank you very much! I will give it a spin later today and report back my results.

@ionitron-bot
Copy link

ionitron-bot bot commented Sep 5, 2018

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 Sep 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants