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 can be encapsulated in a component #28801

Merged
merged 22 commits into from
Jan 15, 2024
Merged

Conversation

liamdebeasi
Copy link
Contributor

@liamdebeasi liamdebeasi commented Jan 9, 2024

Issue number: resolves #16304, resolves #20681


What is the current behavior?

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?

  • 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
  • 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.
  2. 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

Dev build: 7.6.2-dev.11704922151.1fd3369f

@github-actions github-actions bot added the package: core @ionic/core package label Jan 9, 2024
@liamdebeasi liamdebeasi changed the base branch from main to feature-8.0 January 9, 2024 16:22
@liamdebeasi liamdebeasi changed the title Fw 1919 fix(menu): menu can be encapsulated in a component Jan 9, 2024
@liamdebeasi liamdebeasi marked this pull request as ready for review January 10, 2024 22:26
core/src/components/menu/menu.ios.scss Outdated Show resolved Hide resolved
core/src/components/menu/menu.tsx Outdated Show resolved Hide resolved
@liamdebeasi liamdebeasi merged commit 76f6362 into feature-8.0 Jan 15, 2024
44 checks passed
@liamdebeasi liamdebeasi deleted the FW-1919 branch January 15, 2024 20:32
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
Development

Successfully merging this pull request may close these issues.

None yet

2 participants