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: SplitPane not working when Menu is wrapped in a Component #20681

Closed
nickwinger opened this issue Mar 3, 2020 · 16 comments
Closed

bug: SplitPane not working when Menu is wrapped in a Component #20681

nickwinger opened this issue Mar 3, 2020 · 16 comments
Labels
package: core @ionic/core package type: bug a confirmed bug report

Comments

@nickwinger
Copy link

nickwinger commented Mar 3, 2020

Bug Report

When wrappen the menu inside a SpliePane in it's own component (e.g. <my-menu>) the SplitPane is not working as expected (2 Panes are shown)

Ionic version:

[x] 5.x
latest

Current behavior:

When wrapping the menu inside a SplitPane in it's own component (e.g. )
2 Panes are shwon

Expected behavior:

The SplitPane should work as if i would put the <ion-menu> right underneath the SplitPane

Steps to reproduce:

Related code:
not working:

<ion-split-pane when="xl" contentId="main">
  <my-menu></my-menu>

  <ion-router-outlet main id="main"></ion-router-outlet>
</ion-split-pane>

working:

<ion-split-pane when="xl" contentId="main">
  <ion-menu>...</ion-menu>

  <ion-router-outlet main id="main"></ion-router-outlet>
</ion-split-pane>
insert short code snippets here

Other information:

Ionic info:

insert the output from ionic info here
@ionitron-bot ionitron-bot bot added the triage label Mar 3, 2020
@liamdebeasi
Copy link
Contributor

Thanks for the issue. Can you provide a repo with the code required to reproduce this issue?

@liamdebeasi liamdebeasi added the needs: reply the issue needs a response from the user label Mar 3, 2020
@ionitron-bot ionitron-bot bot removed the triage label Mar 3, 2020
@nickwinger
Copy link
Author

Here is the repo where you can reproduce the issue:
https://github.com/nickwinger/ionicMenuBug

The menu should be permanent open on desktop but it isn't

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Mar 5, 2020
@liamdebeasi
Copy link
Contributor

Thanks for the follow up. I need to dig into this more, but at first glance it does not look like this is possible. The ion-menu in your app is missing these styles: https://github.com/ionic-team/ionic/blob/master/core/src/components/split-pane/split-pane.scss#L56-L61.

The problem is that the ::slotted pseudo-element only works on slotted top-level nodes. Since the top level node is your wrapped-menu component, the styles will not get applied.

@nickwinger
Copy link
Author

hmm, i copied the css-classes applied to the ion-menu when it is not wrapped to the wrapped ion-menu but it still not showing.
I guess there must be more into showing the menu but only css-classes (styles) ?
is it really only css classes handling the menu show/hide state ?

@liamdebeasi liamdebeasi added package: core @ionic/core package type: bug a confirmed bug report and removed needs: investigation labels Mar 19, 2020
@liamdebeasi liamdebeasi added this to the 5.0.6 milestone Mar 19, 2020
@liamdebeasi
Copy link
Contributor

Can you try the following dev build and let me know if it fixes the issue?

npm i @ionic/[email protected]

@cnanders
Copy link

cnanders commented Nov 11, 2020

This bug is still happening as of 5.4.3. If you get rid of the<ion-split-pane> it works as advertised with <ion-menu> wrapped in its own component, e.g., <my-side-menu>

<ion-app>
    <my-side-menu></my-side-menu>
    <ion-router-outlet></ion-router-outlet>
</ion-app>

@jbagaresgaray
Copy link

jbagaresgaray commented Apr 7, 2021

This bug is still happening as of 5.6.3 for @ionic/vue

<IonSplitPane content-id="main-content">
     <Sidebar />
     <ion-router-outlet id="main-content" ></ion-router-outlet>
</IonSplitPane>

so the temporary work around is

ion-menu {
  display: flex;
  flex-shrink: 0;
}

@liamdebeasi liamdebeasi removed their assignment Jan 26, 2022
@MRDNZ
Copy link

MRDNZ commented Jan 3, 2023

Bug is still there in "@ionic/angular": "^6.1.4". Is there any update/progress on this subject? It is impossible to wrap split-pane/menu logic in separate components

@timonjagi
Copy link

timonjagi commented Apr 11, 2023

Facing the same issue on @ionic/angular ^6.5.3. Any update on this issue?

@akshay-trex
Copy link

any update on the fix for this bug?

@philjones88
Copy link

Still in v7

@ZenulAbidin
Copy link

Latest version of @ionic/angular ^7.4.3 still has this problem - using href instead of routerLink is an acceptable but not satisfactory workaround.

@arrighidante
Copy link

Still failing in @ionic/angular7.6.1

@liamdebeasi
Copy link
Contributor

liamdebeasi commented Jan 10, 2024

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/[email protected] @ionic/[email protected]

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

liamdebeasi added a commit that referenced this issue Jan 15, 2024
Issue number: resolves #16304, resolves #20681

---------

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

Split Pane assumes that Menu is a child of the Split Pane. This means
that CSS selectors and JS queries only check for children instead of
descendants. When a Menu is encapsulated in a component that component
can itself show up as an element in the DOM depending on the
environment. For example, both Angular and Web components show up as
elements in the DOM. This causes the Menu to not work because it is no
longer a child of the Split Pane.

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

- Menu can now be used as a descendant of Split Pane. The following
changes make this possible:
1. When the menu is loaded, it queries the ancestor Split Pane. If one
is found, it sets `isPaneVisible` depending on if the split pane is
visible or not.
2. Any changes to the split pane visibility state after the menu is
loaded will be handled through the `ionSplitPaneVisible` event.
3. A menu is now considered to be a pane if it is inside of a split pane
4. Related CSS no longer uses `::slotted` which only targets children.
The CSS was moved into the menu stylesheets. I consider the coupling
here to be valuable as menu and split pane are often used together. We
also already have style coupling between the two components, so this
isn't anything new.

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

There are no known changes to the public API or public behavior.
However, there are two risks here:

1. There is an unknown risk into how this change impacts nested split
panes. This isn't an explicitly supported feature, but it technically
works in Ionic now. We don't know if anyone is actually implementing
this pattern. We'd like to evaluate use cases for nested split panes
submitted by the community before we try to account for this
functionality in menu and split pane.
5. There may be some specificity changes due to the new CSS. CSS was
moved from split pane to menu so it could work with encapsulated
components:

`:host(.split-pane-visible) ::slotted(.split-pane-side)` has a
specificity of 0-1-1

`:host(.menu-page-visible.menu-pane-side)` has a specificity of 0-1-0

There shouldn't be any changes needed to developer code since the
selectors are in the Shadow DOM and developer styles are in the Light
DOM. However, we aren't able to anticipate every edge case so we'd like
to place this in a major release just to be safe.

## 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.6.2-dev.11704922151.1fd3369f`
@liamdebeasi
Copy link
Contributor

Thanks for the issue. This has been resolved via #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.

Copy link

ionitron-bot bot commented Feb 14, 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 Feb 14, 2024
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.

10 participants