ionic-team / ionic-framework

A powerful cross-platform UI toolkit for building native-quality iOS, Android, and Progressive Web Apps with HTML, CSS, and JavaScript.
https://ionicframework.com
MIT License
50.93k stars 13.51k forks source link

bug: wrapped menu does not work with content property #16304

Closed iTEEECH closed 8 months ago

iTEEECH commented 5 years ago

Ionic Info

Ionic:

   ionic (Ionic CLI)             : 4.3.1 (/usr/local/lib/node_modules/ionic)
   Ionic Framework               : @ionic/angular 4.0.0-beta.15
   @angular-devkit/build-angular : 0.8.7
   @angular-devkit/schematics    : 0.8.7
   @angular/cli                  : 6.2.7
   @ionic/angular-toolkit        : 1.1.0

System:

   NodeJS : v8.11.3 (/usr/local/bin/node)
   npm    : 6.4.1
   OS     : macOS

Describe the Bug

When putting the menu inside a custom component, (app-menu) to make the app component less busy, I get this result :

https://ibb.co/kUDFOV

But normally, I have to have the initial result :

https://ibb.co/cFcLqA

Steps to Reproduce

1 - Generate a new app with menu 2 - Put the ion-menu in the app-menu `

Menu {{p.title}}

3 - Put the following in app.component:

ion ` Thank you for your answers !
iTEEECH commented 5 years ago

Hi Nobody has any solution for this bug ?

thank you so much !

paulstelzer commented 5 years ago

Thanks for your issue! That seems to be like a bug. A workaround would be this:

<ion-app>
  <ion-split-pane when="lg">
    <ion-menu #leftSidebar menuId="left" type="reveal" side="start">
      <app-sidebar-menu></app-sidebar-menu>
    </ion-menu>

    <ion-router-outlet main></ion-router-outlet>

  </ion-split-pane>
</ion-app>
iTEEECH commented 5 years ago

Thank you for your answer ! So it is impossible to apply the principle of angular and to cut the components ?

paulstelzer commented 5 years ago

Okay, sry. I've updated my answer above. It's a bug, you are right :)

iTEEECH commented 5 years ago

No problem, I will look in my side If I have the time and wait the next version to test :)

paulstelzer commented 5 years ago

I've spent some time with it today. At the moment it's not possible to put it inside a child component in fact of the following:

https://github.com/ionic-team/ionic/blob/master/core/src/components/split-pane/split-pane.tsx#L135

He is looking if the split pane is the parent element. If you put it inside a child component, this will lead to false inside the ion-menu. It can be fixed by looking through another parentElement like so:

  private isPane(element: HTMLElement): boolean {
    if (!this.visible) {
      return false;
    }

    // If element has no parent element, return false
    const parentElement = element.parentElement;
    if (!parentElement) {
      return false;
    }

    // Parent element not matches the element, but it can has another parent element
    // In case the component is inside another component
    if (parentElement && parentElement !== this.el) {
      return this.isPane(parentElement);
    }
    return parentElement === this.el
      && element.classList.contains(SPLIT_PANE_SIDE);
  }

This would lead that it returns true on the ion-menu. But we need to stop this after some circles otherwise he would walk through all elements (not good). But now we have another issue:

https://github.com/ionic-team/ionic/blob/master/core/src/components/split-pane/split-pane.scss#L36

As you can see ion-menu has to be a child of split-pane

Now you could say: And if we remove this child selector? Problem would be, if you have more than one split-pane, then another menu would become visible

So split-pane has to be refactored. So it's a feature request, not a bug :)

WhatsThatItsPat commented 5 years ago

I often have the use case where I have a main root side menu on the left, which is ever-present in the app. Then specific page(s) will have a right "menu."

I hesitate to call the right ones menus; they are sub-functionality for a specific pages (and should be easily accessible from the page's scope/code). It's something that a popover could probably do, but a menu on the right feels better and swiping from the right edge is nice. Maybe there should be a distinction between menus and drawers, with the latter being sub-components of pages.

iTEEECH commented 5 years ago

yes why not ... personally I find the menu component poorly designed.

why not allow to put it in a proper component to him directly ? especially to simplify unit tests.

I take the example of my project where I have several roles and I initialize the menu according to the roles.

I also encounter the problem that I can not activate the menu once authenticated with a * ngIf because the expression false true is already verified because the menu is in the app.component

Eraldo commented 5 years ago

I too am trying to get my "drawers" (side-menus within pages) to work. I was not able to get it to work with version 4.0.0-rc.2:

Ionic:

   ionic (Ionic CLI)             : 4.8.0 (/usr/local/lib/node_modules/ionic)
   Ionic Framework               : @ionic/angular 4.0.0-rc.2
   @angular-devkit/build-angular : 0.12.2
   @angular-devkit/schematics    : 7.2.2
   @angular/cli                  : 7.2.2
   @ionic/angular-toolkit        : 1.2.2

System:

   NodeJS : v11.0.0 (/usr/local/Cellar/node/11.0.0/bin/node)
   npm    : 6.5.0
   OS     : macOS Mojave

I keep getting this error message:

Menu: must have a "content" element to listen for drag events on.
Yogeshjuya1993 commented 5 years ago

Please try.... app.component.html `

Menu menu stuff
<ion-router-outlet id="content" main></ion-router-outlet>

`

Home.html

`

`

https://stackoverflow.com/questions/53657275/ionic-4-adding-side-menu

PurpleEdge2214 commented 5 years ago

I've been trying to put the side menu into a separate component for a couple of days, and found the following seems to work?

In app.component.html ... `

`

In sidemenu.component.html ... `\ \ \ \ \Menu \ \

<ion-content>

  <ion-list>
    <ion-menu-toggle auto-hide="false" *ngFor="let p of appPages">
      <ion-item [routerDirection]="'root'" [routerLink]="[p.url]">
        <ion-icon slot="start" [name]="p.icon"></ion-icon>
        <ion-label>
          {{p.title}}
        </ion-label>
      </ion-item>
    </ion-menu-toggle>
  </ion-list>

</ion-content>

`

i.e. move the entire contents of the split pane to the menu component, including ion-router-outlet.

There is a working example here...

https://github.com/PurpleEdge2214/Ionic4-Side-Menu-as-a-Component---Example

Eraldo commented 5 years ago

@Yogeshjuya1993, @PurpleEdge2214

I don't understand how this solves the issue of wanting to have "drawers"? Meaning: To have different side-menus in different lazy loaded pages. Does it?

(In any way I thank you for sharing your insights.)

liquidcms commented 5 years ago

Is the final answer here that you can't put a side menu is a custom component?

Going from @Yogeshjuya1993's comment i have this in app.component.html

<ion-app>
  <ion-menu contentId="content" side="start">
    <ion-header>
      <ion-toolbar>
        <ion-title>Menu</ion-title>
      </ion-toolbar>
    </ion-header>
    <ion-content> menu stuff </ion-content>
  </ion-menu>
  <ion-router-outlet id="content" main></ion-router-outlet>
</ion-app>

and in my custom header component.html i have this:

<ion-header class="ion-text-center ion-padding">
    <ion-buttons slot="start">
      <ion-menu-button autoHide="false"></ion-menu-button>
    </ion-buttons>
  <img src="../assets/img/header.png">
</ion-header>

then, on every page i want my header (with side menu button), i add this: <app-header></app-header>

it almost works. On the start page when app runs, it works (except menu button is hidden when menu opens); but on every subsequent page, the button shows but doesn't do anything (and no errors); and returning to start page, it also no longer works.

liquidcms commented 5 years ago

Hmm.. i had MenuController imported on one of my pages; this may have been breaking it.

speence commented 5 years ago

It's too bad that the functionality has changed since v3. With a v3 project it was straight forward to have ion-menu components on multiple pages/components of the application. This provided more flexibility with the contents of the menu, as well as functionality. It seems that the new implementation of this component intends for ion-menu to only be used for routing.

I agree with @iTEEECH, I find the way that this component has been designed to be poor. The implementation for v3 provided far more flexibility

ishan123456789 commented 3 years ago

Any update on this? I'm on v5 and I get this error in production but not in development mode. On debugging found that it happens with SSR. So might be related to this issue https://github.com/ionic-team/stencil/issues/2119 And as mentioned there having same content-id removes the error but my menu behaves strangely with no menu visible at all

KANekT commented 2 years ago

bug is actual with https://github.com/ionic-team/ionic-framework/issues/24907

liamdebeasi commented 8 months ago

Hi everyone,

Here is a dev build if anyone is interested in testing a proposed fix: 7.6.2-dev.11704922151.1fd3369f

Install example:

npm install @ionic/react@7.6.2-dev.11704922151.1fd3369f @ionic/react-router@7.6.2-dev.11704922151.1fd3369f

Please note that this is an Ionic 8 build and is subject to the Ionic 8 breaking changes.

liamdebeasi commented 8 months ago

Thanks for the issue. This has been resolved via https://github.com/ionic-team/ionic-framework/pull/28801, and a fix will be available in an upcoming release of Ionic Framework. We are shipping this in a major release of Ionic to de-risk some of the internal infrastructure changes we made to resolve this bug. Feel free to continue testing the dev build, and let me know if you run into any issues.

ionitron-bot[bot] commented 7 months ago

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.