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/Contextual Action Panel: consider adding new styling options #1594

Closed
bartlomiej-k opened this issue Nov 22, 2023 · 8 comments · Fixed by infor-design/enterprise#8702 or #1694
Closed
Assignees
Labels
team: WFM For WFM Issues type: enhancement ✨ [2] Velocity rating (Fibonacci)

Comments

@bartlomiej-k
Copy link

Is your feature request related to a problem? Please describe.
Contextual Action Panel is supposed to be used with large forms or components displayed in a modal. In WFM, we use it to display tens of components of variable, unknown before rendering height. Because rendering so many components takes a long time, we wanted to improve performance by adding some kind of lazy loading/virtual scrolling mechanism, but because it requires setting up the height of the scrolling element, it's impossible to use that in CAP.

The reason for that is the way the max-height and max-width of CAP is calculated and set:

  1. The size is set on div.modal-body-wrapper, which is a grandparent of the component placed inside the panel. This style is not inherited by the actual component wrapper, div.modal-body, so the component doesn't know what the max size is. That means, if we wanted to use this max-width and max-height properties to style the virtual scrolling element, we need to access the styling using document.getElementById(), then find a child, get the styling, parse it, apply, include padding... It's definitely not a way to go.
  2. Even if we decide to do that, the size is calculated in javascript, what can cause a race mechanism between the code executed by modal to calculate its max size and the component placed in the panel which tries to get these values. It's something we observed then we tried to use IntersectionObserver instead of any kind of virtual scrolling components - sometimes it was working correctly, sometimes it was reading the size of the div.modal-body-wrapper before its size was calculated and set, so the height was unrestricted and all elements were rendered.
  3. Even if that was working correctly, there is also a padding: 3rem 0; styling applied to div.modal-body, so that is another thing that needs to be included.

Describe the solution you'd like
Two additional styling options should be added to address these issues:

  • propagateStyle - if true, set a propagate-style class to div.modal-body, which would add additional CSS styling to this element: max-height: inherit; max-width: inherit; overflow: inherit;.
  • compactPanel - if true, set a compact class to div.modal-body, which would add padding: 0; to this element, overriding default padding: 3rem 0; styling for CAP.

Describe alternatives you've considered
For lazy loading/virtual scrolling, we were trying to use:

  • Angular CDK Virtual Scrolling - requires setting the heigth of the element, which leads to problems described above
  • IntersectionObserver - breaks randomly due to race issue described above

For getting the maximum size of the component:

  • document.getElementById() with getElementsByClassName() - race condition described above
  • calculating the size on our own using max-height: calc(80svh - 180px - 6rem) (80% of height - modal height setting - padding to avoid having two scrollbars) - that's a no go as every library upgrade may result with styling being broken and it's not a proper solution if we need to copy the calculations the library does
  • using Action Panel from Web Components library - it works most of the time, but when a component placed inside changes its size after some time (think of fetching the data to display from the server), the panel no longer is centered on the screen - that's a separate issue I need to create 😉

Additional context

  • Infor Application/Team Name: Infor WFM
@tmcconechy tmcconechy added type: enhancement ✨ [5] Velocity rating (Fibonacci) labels Nov 22, 2023
@tmcconechy
Copy link
Member

It sounds like a bad use case for a popup/panel to have thousands of things on it. Also this issue is pretty hard for me to digest.

Would simply adding minWidth, width maxWidth and height options work? Trying to get this in a more approachable way.
Or do we simply need to try to get an example with CDK virtual scroller (or what about our own virtual scroller?) Working

Also if we did do this enhancement i think we would only do it in one place. I.E. the new web component action panel

@tmcconechy tmcconechy added the team: WFM For WFM Issues label Nov 22, 2023
@bartlomiej-k
Copy link
Author

Is there any other modal-like component that we can use in to display hundreds or thousands elements? I thought panel is meant to handle the most complex scenarios.

If these min/max-height and min/max-width were available in any simple and reliable way to the component placed inside the panel, then it would do the work. After all, that's what the max-height: inherit; max-width: inherit; overflow: inherit; applied to the div.modal-body would do. A settings flag is there just to avoid regression.

Having an example for virtual scrolling would also help. Even though in the end we made it work with IntersectionObserver, because of the unknown size of the element, the more user scrolls, the more the 'jumping' there is. That's why I created this ticket - we should use a solution that doesn't have the side effect that makes the feature unusable with 'large enough' dataset.

It's fine to have it in one place only - in WC lib sizing is not an issue, so if that goes to enterprise and enterprise-ng (for panel settings), it would be enough for us.

BTW, is there any Stackblitz example I can fork and use to show the WC's panel positioning issue? I tried simply adding the WC library to the latest Angular example, but couldn't make work and in each attempt I ended up with never-ending compilation.

@tmcconechy
Copy link
Member

tmcconechy commented Nov 22, 2023

I guess the panel is the one that can have the most on it. But when we are talking about putting a thousand fields in a virtual scroller it seemed a bit excessive. But dont have anything else i can recommend other than a simple "page" vs a popup.

Ok lets close this and do whats needed in WC then? If that works for you. Can make an issue there for the positioning issue.

unfortunately seems stackblitz doesnt work with Es modules so i was unable to get that working. What i have been suggesting is to take this simple HTML project https://github.com/infor-design/enterprise-wc-examples/tree/main/plain-html and create it in there, then zip it up and send.

@bartlomiej-k
Copy link
Author

We still have some problems with adopting WC in WFM and I think we won't be able to switch to the new library in a close future, so I prefer having it fixed in enterprise and enterprise-ng. Especially that adding a new, conditionally applied styling seems easier (or quicker to implement) than fixing positioning issue 😉

I will try to use this example instead of a Stackblitz. Thanks!

@tmcconechy
Copy link
Member

Ok if its just the size settings we had planned that on this issue anyways infor-design/enterprise#8098

@bartlomiej-k
Copy link
Author

Haven't seen that one, but it looks like my ticket is mostly a duplicate of the linked one. Could there be also an option to remove padding? That would cover all points from this ticket.

@tmcconechy
Copy link
Member

tmcconechy commented Mar 27, 2024

@bartlomiej-k do you need this in NG or Web Components? Or both?

@bartlomiej-k
Copy link
Author

Currently we use only NG, but we have a plan to migrate to WC, so it would be best to have it in both with NG being a higher priority.

@AAlviar AAlviar added [2] Velocity rating (Fibonacci) and removed [5] Velocity rating (Fibonacci) labels Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team: WFM For WFM Issues type: enhancement ✨ [2] Velocity rating (Fibonacci)
4 participants