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: navigating with multiple split panes causes first menu to never be reactivated #18683

Closed
yogeshshsf opened this issue Jul 1, 2019 · 16 comments · Fixed by #28370
Closed
Labels
package: core @ionic/core package type: bug a confirmed bug report

Comments

@yogeshshsf
Copy link

yogeshshsf commented Jul 1, 2019

Hi. i am using 2 split-pane, firstmenu/firstpage will be my home screen. from firstpage i am navigating to secondmenu/secondpage. now when i return back to firstmenu/firstpage, the split-pane is getting hide, can someone help me on this please. see the screenshot and code below.

firstmenu/firstpage:

image

secondmenu/secondpage:

image

firstmenu/firstpage:

image

Below is the github repository to test this:

https://github.com/yogeshshsf/SplitpaneDemo

@ionitron-bot ionitron-bot bot added the triage label Jul 1, 2019
@yogeshshsf
Copy link
Author

Hi. i am using 2 split-pane, firstmenu/firstpage will be my home screen. from firstpage i am navigating to secondmenu/secondpage. now when i return back to firstmenu/firstpage, the split-pane is getting hide, can someone help me on this please. see the screenshot and code below.

Members
0
18 posts
Report post
Posted May 29 (edited)
Hi simon. i am using 2 split-pane, firstmenu/firstpage will be my home screen. from firstpage i am navigating to secondmenu/secondpage. now when i return back to firstmenu/firstpage, the split-pane is getting hide. this is stopper for my application, can you help on this please. see the screenshot and code below.

firstmenu/firstpage:

image

@liamdebeasi
Copy link
Contributor

Thanks for the issue. I think this problem stems from the fact that we only allow 1 menu to be enabled at a time. If you want these two side panes to be always be open, I would recommend wrapping them in something other than ion-menu.

In your demo repo, I wrapped them each in a div rather than an ion-menu and the issue disappeared. Can you try this and let me know if it works for your use case?

If you really need to use ion-menu, I would recommend only having 1 instance of the menu.

@liamdebeasi liamdebeasi added the needs: reply the issue needs a response from the user label Jul 2, 2019
@ionitron-bot ionitron-bot bot added triage and removed triage needs: reply the issue needs a response from the user labels Jul 2, 2019
@liamdebeasi liamdebeasi added the needs: reply the issue needs a response from the user label Jul 8, 2019
@ionitron-bot ionitron-bot bot removed the triage label Jul 8, 2019
@eduardoRoth
Copy link
Contributor

Issue happens even if only using one ion-menu; @liamdebeasi your quick fix works, if I replace ion-menu tag with a div one, split pane is always shown.

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Jul 12, 2019
@yogeshshsf
Copy link
Author

@liamdebeasi thanks for you help, its much appreciated. it works perfect when you use div..

@eduardoRoth
Copy link
Contributor

When using div tag instead of ion-menu; tags ion-content and ion-footer don't work as expected.

@iamkinetic
Copy link

iamkinetic commented Aug 7, 2019

@liamdebeasi are you saying we should have no more than one menu in an application or we may run into problems even if only one of them is shown at one time?

@liamdebeasi
Copy link
Contributor

liamdebeasi commented Aug 7, 2019

No, but in the case of the original issue the way the multiple menus were added seemed to cause issues. We are investigating this bug and will post an update here when we have an update. Thanks!

@daveshirman
Copy link

@yogeshshsf Any luck on this?

@yogeshshsf
Copy link
Author

@daveshirman it is working as expected when i wrap using

as mentioned by @liamdebeasi comment

@Fab4guy
Copy link

Fab4guy commented Jul 7, 2020

I have exactly the same problem.

I want to outsource my navigation in a layout component.
I have done all that so far. For testing I created two pages, Home and Contact.
When I load the application, everything is displayed correctly. If I now click on the navigation point Contact, the page changes as expected. If I click on the Home menu item again, the page changes, but the navigation on the left side disappears forever.

I hope you can help me

Here the structure of the different files. I also attach a picture of the folder structure

folder

Sorry but somehow the editor does not want to highlight my code

app-routing.module.ts

`import { NgModule } from '@angular/core';
import { PreloadAllModules, RouterModule, Routes } from '@angular/router';
import {MainLayoutWithSideNavComponent} from './layout/main-layout-with-side-nav/main-layout-with-side-nav.component';

const routes: Routes = [
{
path: '',
redirectTo: 'home',
pathMatch: 'full',
canActivate: [],
},
{
path: 'home',
component: MainLayoutWithSideNavComponent,
canActivate: [],
loadChildren: () => import('./pages/home/home.module').then(m => m.HomePageModule)
},
{
path: 'contact',
component: MainLayoutWithSideNavComponent,
canActivate: [],
loadChildren: () => import('./pages/contact/contact.module').then(m => m.ContactPageModule)
}
];

@NgModule({
imports: [
RouterModule.forRoot(routes, { preloadingStrategy: PreloadAllModules })
],
exports: [RouterModule]
})
export class AppRoutingModule {}
`
main-layout-with-side-nav.component.html

`

Menu
<ion-content>
  <ion-list>
    <ion-list-header>
      Navigate
    </ion-list-header>
    <ion-menu-toggle auto-hide="false">

      <ion-item *ngFor="let nav of navi" button [routerLink]="'/'+ nav.link">
        <ion-icon slot="start"></ion-icon>
        <ion-label>
          {{nav.linkName}}
        </ion-label>
      </ion-item>

    </ion-menu-toggle>
  </ion-list>
</ion-content>

`

app.component.html

<ion-app> <ion-router-outlet animated="false"></ion-router-outlet> </ion-app>

@liamdebeasi liamdebeasi is there a solution?

@liamdebeasi liamdebeasi changed the title [ionic v4] ion-split-pane disappears on page change bug: navigating with multiple split panes causes first menu to disappear Oct 13, 2020
@liamdebeasi liamdebeasi changed the title bug: navigating with multiple split panes causes first menu to disappear bug: navigating with multiple split panes causes first menu to never be reactivated Oct 13, 2020
@liamdebeasi liamdebeasi added package: core @ionic/core package type: bug a confirmed bug report labels Oct 13, 2020
@ionitron-bot ionitron-bot bot removed the triage label Oct 13, 2020
@thomaux
Copy link

thomaux commented Apr 6, 2023

I fixed this by replacing the <ion-menu> with a div as mentioned before. Then, in order to make <ion-header> and <ion-content> work properly inside that div, I applied flex-direction: column; to the div. Hope this helps!

@cra1gs
Copy link

cra1gs commented Apr 14, 2023

I fixed this by replacing the <ion-menu> with a div as mentioned before. Then, in order to make <ion-header> and <ion-content> work properly inside that div, I applied flex-direction: column; to the div. Hope this helps!

If you use this method, how does your menu toggle work on smaller screens? My burger menu will not appear, so you cannot access the menu.

@thomaux
Copy link

thomaux commented Apr 14, 2023

@cra1gs this is how I ended up using the split-pane:

<ion-split-pane when="xs" contentId="location-specifics" [disabled]="closeMenu$ | async">
  <div>
    <ion-content>
      <ion-list>
        ...menu content here
      </ion-list>
    </ion-content>
  </div>

  <ion-router-outlet id="location-specifics"></ion-router-outlet>
</ion-split-pane>
export class SplitPanePage {

  closeMenu$ = this.store.selectedItem$.pipe(
    map((val) => val && window.innerWidth <= 576),
  );
}
@media (max-width: 576px) {
  ion-split-pane {
    --side-min-width: 100%;
  }
}

The basic premise of the above is: when an item is selected AND the screen is smaller than the SM breakpoint (=576px), disable the split screen so the "menu" is not shown. In case no item is selected, only the menu is shown as the when property is set to xs and I've set the side minimum width to 100% for screen widths lower then the SM breakpoint

@liamdebeasi liamdebeasi added type: bug a confirmed bug report and removed type: bug a confirmed bug report labels Oct 5, 2023
liamdebeasi added a commit that referenced this issue Oct 10, 2023
Issue number: resolves #18974

---------

<!-- 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?
<!-- Please describe the current behavior that you are modifying. -->

When multiple menus on the same `side` are registered, all but the most
recent menu are disabled. For example, if a user starts on PageA with a
`start` menu and then navigates to PageB which also has a `start` menu,
then the menu on PageA will be disabled. The problem is that if users
navigates back to PageA they will be unable to open the menu on that
view because it is still disabled. This behavior impacts any Ionic
developer trying to open a menu whether by calling the `open` method on
the menu itself or on the `menuController`.

After discussing with the team, we believe the original intent of this
behavior was to prevent users from accidentally opening the wrong menu
when calling `menuController.open('start')`. This API allows developers
to reference a menu by side, and since it's possible to have multiple
menus on the same side it's also possible to open the wrong menu when
referencing by side only.

However, this API starts to break down pretty quickly in a navigation
scenario.

Sample Repo: https://github.com/liamdebeasi/multiple-menu-bug-repro

## Scenario 1: Referencing Menu by Side

1. On the "home" route click "Open 'start' menu". Observe that the home
page menu opens.
2. Close the menu and click "Go to Page Two".
3. On the "page-two" route click "Open 'start' menu". Observe that the
page two menu opens.
4. Go back to "home".
5. Click "Open 'start' menu". Observe that nothing happens.
6. Click "Enable and Open 'start'" Menu". Observe that the home menu
opens.

## Scenario 2: Referencing Menu by ID

1. On the "home" route click "Open '#menu1' menu". Observe that the home
page menu opens.
2. Close the menu and click "Go to Page Two".
3. On the "page-two" route click "Open '#menu2' menu". Observe that the
page two menu opens.
4. Go back to "home".
5. Click "Open '#menu1' menu". Observe that nothing happens.
6. Click "Enable and Open '#menu1'" Menu". Observe that the home menu
opens.

## Scenario 3: Using 3 or more menus even when enabling menus

1. On the "home" route click "Open 'start' menu". Observe that the home
page menu opens.
2. Close the menu and click "Go to Page Two".
3. On the "page-two" route click "Open 'start' menu". Observe that the
page two menu opens.
4. Close the menu and click "Go to Page Three"
5. On the "page-three" route click "Open 'start' menu". Observe that the
page three menu opens.
6. Go back to "page-two".
8. Click "Open 'start' menu". Observe that nothing happens.
9. Click "Enable and Open 'start' Menu". Observe that nothing happens.

The menu controller attempts to find an enabled menu on the specified
side:
https://github.com/ionic-team/ionic-framework/blob/a04a11be3571faa99c751edc034462e94a977e95/core/src/utils/menu-controller/index.ts#L79C12-L79C12

Step 6 is where this breaks down. In this scenario, the menus on "home"
and "page-two" are disabled. This leads menu controller to use its
fallback which tries to get the first menu registered on the specified
side:
https://github.com/ionic-team/ionic-framework/blob/a04a11be3571faa99c751edc034462e94a977e95/core/src/utils/menu-controller/index.ts#L86

This means that the menu controller would attempt to open the "home"
menu even though the user is on "page-two" (because the start menu on
"home" was the first to be registered).

## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

- Menus are no longer automatically disabled when a new menu on the same
side is registered
- Referencing menus by side when multiple menus with that side exist in
the DOM will cause a warning to be logged

This change has a couple implications:

1. Developers no longer need to manually enable a menu as noted in
https://ionicframework.com/docs/api/menu#multiple-menus. Note that
continuing to manually enable the menus will not cause any adverse side
effects and will effectively be a no-op.
2. Developers using the menuController to open a menu based on "side"
may end up having the wrong menu get opened.

Example before to this change:

1. Start on PageA with a `start` menu. Calling
`menuController.open('start')` opens the menu on PageA.
2. Go to PageB with a `start` menu. Calling
`menuController.open('start')` opens the menu on PageB because the menu
on PageA is disabled.

Example after to this change:

1. Start on PageA with a `start` menu. Calling
`menuController.open('start')` opens the menu on PageA.
2. Go to PageB with a `start` menu. Calling
`menuController.open('start')` attempts to opens the menu on PageA
because both menus are enabled. However, since PageA is hidden nothing
will appear to happen.

## 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

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->

I manually verified that removing the Angular Universal code does not
regress the behavior fixed in
#27814. The menu is
never automatically disabled, so the bug does not happen.

This is a partial fix for
#18683. Properly
fixing this requires another change which is out of scope for this work.
@liamdebeasi
Copy link
Contributor

Hi everyone,

Here is a dev build with a proposed fix if anyone is interested in testing: 7.5.1-dev.11697568647.1ac87d08

Install Example: npm install @ionic/angular@7.5.1-dev.11697568647.1ac87d08

github-merge-queue bot pushed a commit that referenced this issue Oct 19, 2023
Issue number: resolves #18683, resolves #15538, resolves #22341

---------

<!-- 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?
<!-- Please describe the current behavior that you are modifying. -->

Menus in a split pane are hidden when a second split pane is
mounted/made visible. This is because the `onSplitPaneChanged` callback
does not take into account whether the it is a child of the split pane
that emitted `ionSplitPaneVisible`.

When split pane 2 is shown, that causes the menu is split pane 1 to
hide. When split pane 1 is shown, the menu inside of it _is_ shown.
However, since split pane 2 is then hidden that component also emits
`ionSplitPaneVisible`, causing the menu inside of split pane 1 to hide.

## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

- Menus are only hidden when its parent split pane changes visibility

## 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

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->

Dev build: `7.5.1-dev.11697568647.1ac87d08`

---------

Co-authored-by: Amanda Johnston <90629384+amandaejohnston@users.noreply.github.com>
@liamdebeasi
Copy link
Contributor

Thanks for the issue. This has been resolved via #28370, and a fix will be available in an upcoming release of Ionic Framework. Please feel free to keep testing the dev build, and let me know if you run into any problems.

sean-perkins pushed a commit that referenced this issue Oct 27, 2023
Issue number: resolves #18683, resolves #15538, resolves #22341

---------

<!-- 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?
<!-- Please describe the current behavior that you are modifying. -->

Menus in a split pane are hidden when a second split pane is
mounted/made visible. This is because the `onSplitPaneChanged` callback
does not take into account whether the it is a child of the split pane
that emitted `ionSplitPaneVisible`.

When split pane 2 is shown, that causes the menu is split pane 1 to
hide. When split pane 1 is shown, the menu inside of it _is_ shown.
However, since split pane 2 is then hidden that component also emits
`ionSplitPaneVisible`, causing the menu inside of split pane 1 to hide.

## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

- Menus are only hidden when its parent split pane changes visibility

## 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

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->

Dev build: `7.5.1-dev.11697568647.1ac87d08`

---------

Co-authored-by: Amanda Johnston <90629384+amandaejohnston@users.noreply.github.com>
Copy link

ionitron-bot bot commented Nov 18, 2023

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

Successfully merging a pull request may close this issue.

8 participants