enterprise-ng icon indicating copy to clipboard operation
enterprise-ng copied to clipboard

Modal/Contextual Action Panel: consider adding new styling options

Open bartlomiej-k opened this issue 2 years ago • 8 comments

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

bartlomiej-k avatar Nov 22 '23 14:11 bartlomiej-k

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 avatar Nov 22 '23 14:11 tmcconechy

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.

bartlomiej-k avatar Nov 22 '23 15:11 bartlomiej-k

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.

tmcconechy avatar Nov 22 '23 15:11 tmcconechy

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!

bartlomiej-k avatar Nov 22 '23 15:11 bartlomiej-k

Ok if its just the size settings we had planned that on this issue anyways https://github.com/infor-design/enterprise/issues/8098

tmcconechy avatar Nov 22 '23 15:11 tmcconechy

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.

bartlomiej-k avatar Nov 22 '23 15:11 bartlomiej-k

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

tmcconechy avatar Mar 27 '24 13:03 tmcconechy

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.

bartlomiej-k avatar Mar 27 '24 14:03 bartlomiej-k