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: #12948 || Overlay: p-overlay div not removed from DOM on hiding … #13659

Conversation

ashikjs
Copy link
Contributor

@ashikjs ashikjs commented Sep 12, 2023

Fix: #12948

Previously I did try to solve it by z-index for CSS animation but now I re-check hole process and identified that dome should remove when overlay is hide.

There issue was after complete animation this.modalVisible = false; can not event fire angular change detection life cycle so dome was not removed properly so we fire it manually by this.cd.markForCheck() then dome gone and solve it issues.

Extra code:

I removed event from (click)="onOverlayClick($event)" because onOverlayClick function don't have any parameter.

@vercel
Copy link

vercel bot commented Sep 12, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
primeng-ssr-test ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 27, 2023 0:14am
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
primeng ⬜️ Ignored (Inspect) Visit Preview Sep 27, 2023 0:14am

@ashikjs
Copy link
Contributor Author

ashikjs commented Sep 12, 2023

@mertsincan and @gucal hi, can you please check this PR. Thanks in advance.

Copy link
Contributor

@SoyDiego SoyDiego left a comment

Choose a reason for hiding this comment

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

I don't know if this is related with a previous PR that I did. Please check if it's possible #13501

Thanks

@gucal gucal self-requested a review September 12, 2023 13:33
@ashikjs
Copy link
Contributor Author

ashikjs commented Sep 12, 2023

@SoyDiego Thanks for your concern. I checked PR and Issues you mentions. those are not relevent to this PR.

Actually here I am ensuring that default overlay z-index is negative so it shouldn't do any side effect to others.
Previously overlap was not any z-index.

@SoyDiego
Copy link
Contributor

@SoyDiego Thanks for your concern. I checked PR and Issues you mentions. those are not relevent to this PR.

Actually here I am ensuring that default overlay z-index is negative so it shouldn't do any side effect to others. Previously overlap was not any z-index.

Thanks for double check 👌

@cetincakiroglu
Copy link
Contributor

Hi @ashikjs,

We cannot set z-index manually in this case we've got a utility function called zIndexUtils in components/utils, could you please remove the manual CSS and use zIndexUtils? You can check the sidebar or dialog to see how we manage zIndex by using zIndexUtils.

@cetincakiroglu cetincakiroglu added the Resolution: Needs Revision The pull request can't be merged. Conflicts need to be corrected or documentation and code updated. label Sep 25, 2023
@ashikjs
Copy link
Contributor Author

ashikjs commented Sep 25, 2023

@cetincakiroglu ok let me try.

@ashikjs
Copy link
Contributor Author

ashikjs commented Sep 25, 2023

@cetincakiroglu can you check my PR now. I wrote details about my latest change in description box.

@ashikjs ashikjs force-pushed the issues-12948/overlap-p-overlay-mobile-view-issue branch 4 times, most recently from c11764b to adf226e Compare September 27, 2023 11:49
@ashikjs ashikjs force-pushed the issues-12948/overlap-p-overlay-mobile-view-issue branch 2 times, most recently from 5d30d6f to e823806 Compare September 27, 2023 12:05
@ashikjs
Copy link
Contributor Author

ashikjs commented Nov 6, 2023

@gucal and @cetincakiroglu can you check my PR.

@cetincakiroglu cetincakiroglu removed the Resolution: Needs Revision The pull request can't be merged. Conflicts need to be corrected or documentation and code updated. label Nov 8, 2023
@cetincakiroglu cetincakiroglu merged commit d1cf02f into primefaces:master Nov 8, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Overlay: p-overlay div not removed from DOM on hiding panel
3 participants