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 angular 7.5, cannot access menus with menuController #28337

Closed
3 tasks done
rvcroffi opened this issue Oct 11, 2023 · 4 comments · Fixed by #28343
Closed
3 tasks done

bug: ionic angular 7.5, cannot access menus with menuController #28337

rvcroffi opened this issue Oct 11, 2023 · 4 comments · Fixed by #28343
Labels
package: angular @ionic/angular package type: bug a confirmed bug report

Comments

@rvcroffi
Copy link

Prerequisites

Ionic Framework Version

v7.x

Current Behavior

After upgrade @ionic/angular from 7.4.4 to 7.5.0 MenuController.enable() stop working.

Expected Behavior

A menu initially disabled should be enabled after calling enable(true) from MenuController on app.component.ts.

Steps to Reproduce

  1. Create a new Ionic sidemenu app (angular).
  2. Run npm i @ionic/[email protected]
  3. Disable ion-menu on app.component.html <ion-menu contentId="main-content" type="overlay" [disabled]="true">
  4. Enable the menu on app.component.ts when platform is ready:
constructor(private menuCtrl: MenuController, private platform: Platform) {
    this.platform.ready().then(() => {
      this.menuCtrl.enable(true);
    });
  }
  1. Run ionic serve and you will see the menu enabled (as expected).
  2. Stop ionic serve and run npm i @ionic/[email protected]
  3. Run ionic serve again and the menu will be disabled.

Code Reproduction URL

https://stackblitz.com/edit/angular-lgwu3i?file=src%2Fapp%2Fapp.component.ts

Ionic Info

Ionic:

Ionic CLI : 7.1.1
Ionic Framework : @ionic/angular 7.5.0
@angular-devkit/build-angular : 16.2.6
@angular-devkit/schematics : 16.2.6
@angular/cli : 16.2.6
@ionic/angular-toolkit : 9.0.0

Capacitor:

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

Utility:

cordova-res : 0.15.4
native-run : 1.7.3

System:

NodeJS : v18.16.0 (C:\Program Files\nodejs\node.exe)
npm : 9.6.7
OS : Windows 10

Additional Information

No response

@liamdebeasi
Copy link
Contributor

Thanks for reporting this! I can reproduce the reported behavior. The problem is with the introduction of our standalone work, we now unintentionally create separate instances of the menuController. So the menu itself is registered with menuController A, but the application is using menuController B which is why enable does not appear to work. (since menuController B does not know about the menus in menuController A)

As a temporary workaround, you can use menuController from @ionic/core instead of MenuController from @ionic/angular. I'll work on a fix for this.

@liamdebeasi liamdebeasi changed the title bug: enable menu is not working on @ionic/[email protected] bug: ionic angular 7.5, cannot access menus with menuController Oct 12, 2023
@liamdebeasi liamdebeasi added package: angular @ionic/angular package type: bug a confirmed bug report labels Oct 12, 2023
@ionitron-bot ionitron-bot bot removed the triage label Oct 12, 2023
@liamdebeasi
Copy link
Contributor

liamdebeasi commented Oct 12, 2023

Here's a dev build if you would like to test a proposed fix:

npm install @ionic/[email protected]

@liamdebeasi liamdebeasi removed their assignment Oct 12, 2023
github-merge-queue bot pushed a commit that referenced this issue Oct 12, 2023
Issue number: resolves #28337

---------

<!-- 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. -->

Duplicate instances of `menuController` are being created in
`@ionic/angular`. `ion-menu` registers itself in the `menuController`
from `@ionic/core`, but the `MenuController` from `@ionic/angular` uses
the `menuController` from `@ionic/core/components`. This is how the
overlay providers work too. Normally, this is not a problem. However,
`menuController` caches references to registered menus in each
controller instances:
https://github.com/ionic-team/ionic-framework/blob/dcbf45101f4c0dc3541cd4c19186daa3766a6ce7/core/src/utils/menu-controller/index.ts#L14

This means that since there are two different controllers,
`menuController` B does not know about the menus in `menuController` A.
The end result is that the menu controller used in developer
applications did not have references to the registered menus, which gave
the impression that the menu controller did not work.

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

- Updated the architecture of `MenuController` in Ionic Angular to
accept a `menuController` instance. This allows `@ionic/angular` to pass
the `menuController` from `@ionic/core` and for
`@ionic/angular/standalone` to pass the `menuController` from
`@ionic/core/components`.

Note: Overlay controllers don't **need** this change per-se since they
don't cache references to overlays internally (they just query the DOM).
However, I think it would be good to have a consistent architecture
here, so I'll put up a separate PR that makes this change for overlays
too.

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

Dev build: `7.5.1-dev.11697123035.1ee6b4a2`

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

Thanks for the issue. This has been resolved via #28343, and a fix will be available in an upcoming release of Ionic Framework. Feel free to continue testing the dev build, and let me know if you have any questions!

Copy link

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

Successfully merging a pull request may close this issue.

2 participants