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

Modal: Does not center correctly on scrolled pages #8887

Open
andybritton-info opened this issue Jul 10, 2024 · 6 comments
Open

Modal: Does not center correctly on scrolled pages #8887

andybritton-info opened this issue Jul 10, 2024 · 6 comments
Assignees
Labels
type: bug 🐛 [3] Velocity rating (Fibonacci)

Comments

@andybritton-info
Copy link

Describe the bug
When a modal is displayed on a scrolled page, it does not correctly center vertically.

To Reproduce

Steps to reproduce the behavior:

  1. Go to 'demo page'
  2. Scroll down the page until the 'Show Modal' button is at least half way up the browser window
  3. Click on 'Show Modal'
  4. Observe that the modal is not vertically centered

Expected behavior
The modal should be in the middle of the browser window, regardless of the scrolling position of the page.

Version

  • ids-enterprise: v4.96.0

Screenshots
image

Platform

  • Infor Application/Team Name: Infor Pathway
  • Device: Windows PC
  • OS Version: Windows 11
  • Browser Name: Chrome, Firefox

Additional context
Firstly, I note that the div.modal-page-container is positioned relative to the initial containing block rather than the viewport.
image
Overriding the above with:
image
... results in a correctly centered modal regardless of page scroll position:
image
The above explains the behavior but I don't know if it's good enough to propose as a fix.

Secondly, I note that this only really came to light because I had a modal with no 'focusable' controls on it and a page that was twice the height of the viewport. As long as there is a focusable control, and as long as the modal code correctly identifies it and executes .focus against it, then the browser will scroll the control (and often the entire modal) back into view. In my case, I had a modal where the only focusable controls where the buttons on the .modal-buttonset, but I had these disabled at the point the modal was being displayed. The modal code was incorrectly trying to focus one of these buttons, but as they were disabled, the browser ignored the event and did not scroll the page to bring the modal into view.

@andybritton-info andybritton-info changed the title Modal does not center correctly on scrolled pages Modal: Does not center correctly on scrolled pages Jul 10, 2024
@tmcconechy tmcconechy added type: bug 🐛 [3] Velocity rating (Fibonacci) labels Jul 10, 2024
@tmcconechy
Copy link
Member

@andybritton-info noting for future dev that it works ok on https://main-enterprise.demo.design.infor.com/components/modal/test-body-scroll.html

And perhaps that could help with a workaround until we get to it

@andybritton-info
Copy link
Author

Ah yes - I see how that works in the demo now. thanks

@yohannahbautista
Copy link
Contributor

@andybritton-info @tmcconechy @ericangeles
Hello, I've been trying to reproduce the bug in IDS locally. I was able to do it by clearing everything in https://github.com/infor-design/enterprise/blob/main/app/views/layout.html and leaving only {{> content}} inside the <body> (all classes in body should be removed). The fix mentioned above works, but also given that I had to remove most of the main template for IDS, I'm not sure if a fix is needed or just extra steps to implement it in stackblitz.

I managed to make it work by adding what was in layout.html: https://stackblitz.com/edit/stackblitz-starters-ibfjke?file=index.html

@techabilla
Copy link

@yohannahbautista, perhaps it is enough to note in documentation that the modal positions itself absolutely and relies on having a non-scrollable, positioned ancestor to anchor it. That said, it might make it more difficult when introducing the component into an existing project, as was the case for me.

@tmcconechy
Copy link
Member

@yohannahbautista i agree maybe add a note in the docs and lets go with this. We have a lot of other items to address.

@tmcconechy
Copy link
Member

Does that means we close with the stackblitz or let QA look at it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug 🐛 [3] Velocity rating (Fibonacci)
Projects
Status: Ready For QA PR Review
Development

No branches or pull requests

4 participants