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

fix(menu): menu no longer disappears with multiple split panes #28370

Merged
merged 7 commits into from
Oct 19, 2023

Conversation

liamdebeasi
Copy link
Contributor

@liamdebeasi liamdebeasi commented Oct 17, 2023

Issue number: resolves #18683, resolves #15538, resolves #22341


What is the current behavior?

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?

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

Does this introduce a breaking change?

  • Yes
  • No

Other information

Dev build: 7.5.1-dev.11697568647.1ac87d08

@github-actions github-actions bot added the package: core @ionic/core package label Oct 17, 2023
@liamdebeasi liamdebeasi changed the title Fw 5299 fix(menu): menu no longer disappears with multiple split panes Oct 17, 2023
@liamdebeasi liamdebeasi marked this pull request as ready for review October 17, 2023 20:40
core/src/components/split-pane/test/multiple/index.html Outdated Show resolved Hide resolved
</ion-menu>
</ion-split-pane>

<ion-split-pane id="pane-two" content-id="content-two" disabled="true" class="ion-page-hidden">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unsure if this is related to this PR, but there seems to be something strange going on with the menus when trying to open them with gestures. At least one of the four menus can't be opened, and sometimes leads to errors in the console while dragging. It seems inconsistent which one(s); I tried a few more refreshes after the below recording and got different results.

2023-10-19.09-56-33.mp4

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that's this issue: #19410

The menus all get added to the same document, so there's an inconsistency in the order in which they open because it's dependent on which menus registers first. For example, if you refresh a few times you'll eventually find that the menu you could open before no longer opens. In reality, a menu that is hidden is being activated instead. However, if that menu has display: none then it will have a width of 0, resulting in the animation error (because we're dividing by 0 when computing the gesture step).

@lincolnthree
Copy link

Nice! Now I can re-design my entire app and hopefully get rid of my custom side-panel component :D Thanks, @liamdebeasi ! This is the best kind of "now I have more work to do" :)

sean-perkins pushed a commit that referenced this pull request 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 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: core @ionic/core package
Projects
None yet
3 participants