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(module:cdk): zIndex is not used properly when creating overlay #8373

Conversation

ParsaArvanehPA
Copy link
Contributor

@ParsaArvanehPA ParsaArvanehPA commented Jan 27, 2024

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[✔] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Application (the showcase website) / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

The modal, drawer, or any other components that are made with the Angular CDK have the zIndex applied to the inner element of the created overlay. This is not desirable for two reasons:

  1. The created overlay is a direct child of the body element
  2. Each overlay has its own wrapper with the class name 'cdk-global-overlay-wrapper'
    Therefore, changing the zIndex of the inner element has no effect, as it is already isolated from other overlays. The expected behavior is that the zIndex should be applied to the overlay wrapper instead.

The image below illustrates this issue:

  • The red sections are the overlay wrappers

  • The green section is the inner element of the overlay wrapper, which has its zIndex changed

The scenario is as follows:
We have a modal component, which contains a button that opens a drawer component (the first red outline is the drawer wrapper, the second is the modal wrapper).
As stated in the issue below, the drawer component should appear above the modal component; but currently, the modal component is on top. The reason why the zIndex property does not work is that it is applied to the inner element of the overlay, rather than the overlay wrapper.

This issue occurs whenever the CDK overlay is used; currently, only modal and drawer components have been fixed. If this pull request is approved, I will update other components that use the overlay as well.

Issue Number: #8353 , #8336

What is the new behavior?

Mentioned above.

Does this PR introduce a breaking change?

[ ] Yes
[✔] No

Other information

Currently it is not possible to set overlays' zIndex value when creating it (issue).

image

Copy link

zorro-bot bot commented Jan 27, 2024

This preview will be available after the AzureCI is passed.

Copy link

codecov bot commented Jan 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6024bcc) 91.74% compared to head (6b3072a) 91.76%.
Report is 21 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8373      +/-   ##
==========================================
+ Coverage   91.74%   91.76%   +0.02%     
==========================================
  Files         520      520              
  Lines       18019    18031      +12     
  Branches     2838     2839       +1     
==========================================
+ Hits        16531    16546      +15     
+ Misses       1185     1183       -2     
+ Partials      303      302       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@Nicoss54 Nicoss54 left a comment

Choose a reason for hiding this comment

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

Please could you provide test for the code added ?

@ParsaArvanehPA
Copy link
Contributor Author

Please could you provide test for the code added ?

Sure, I will get back to you with that as soon as I can.

@ParsaArvanehPA
Copy link
Contributor Author

Please could you provide test for the code added ?

Sorry for the delay; tests for the changes have been added.
Thank you for reviewing my PRs @Nicoss54

Copy link
Collaborator

@Nicoss54 Nicoss54 left a comment

Choose a reason for hiding this comment

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

LGTM

@simplejason simplejason merged commit b932d65 into NG-ZORRO:master Mar 11, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants