Analytical Table: allow to disable virtualization / respect sub components in height calculation

[texttechne]

The Problem

Expanding subComponents won't affect the height calculation of the AnalyticalTable. This becomes a problem when only a few items are rendered.

Here's a sandbox demonstrating the issue.

Use Case

For us this becomes relevant, because of nesting multiple Analytical Tables, so that each level has it's own table with it's own column definition.

• Level 1: Many records guaranteed
  • Works perfectly!
  • Virtualization is desired
  • no problem to render additional empty rows (not optimal, though)
• Level 2: Only a few items will ever get rendered
  • no empty rows can be shown: minRows={1}
  • height of this table is fixed to / dependend on amount of rows
  • problem: expanding sub components doesn't change height of this table
• Level 3: Possibly many items
  • BUG: user has to scroll within the restricted height of the table of level 2

Describe the solution you'd like

I would suggest an option to disable virtualization.

Reasoning: While virtualization is a pretty cool technique (and it's pretty awesome that you've already integrated that stuff into the component!!!), it's only needed if you have hundreds of records. And virtualization has this one drawback (not counting any performance overhead and the like): The height of the table needs either to be fixed or calculated.

And this is exactly my problem: I want the AnalyticalTable to simply take up the space it needs!

Describe alternatives you've considered

Make the height calculation respect expanded sub components.

[texttechne]

After deep diving into the code & the problem itself, I've tried to correct the height calculation: See https://github.com/SAP/ui5-webcomponents-react/pull/1706

[Lukas742]

Hi @texttechne

thanks again for creating the PR :)

You can actually kind of disable vertical virtualisation by setting the overscanCount to a very high number like Infinity.

[JH-nAG]

I encountered the same issue. What @texttechne did in his PR sounded useful. Please fix it. Thank you

[Lukas742]

Hi all,

unfortunately we now decided against implementing the feature mentioned here, for the following reasons:

Disabling virtualization:

Dynamically removing the logic of the virtualisation is not practical as its deeply implemented in the table core and cannot be controlled by hooks. Refactoring this would result in a huge effort on our end and even with excessive testing, we probably wouldn't catch all edge cases. So, if you want to disable virtualisation, please set the overscanCount or the overScanCountHorizontal to a very high number. This way all rows and columns will be rendered into the DOM.

Respecting subcomponents in height calculation

Since v1.19.0 you can use the subComponentsBehavior prop to define how subcomponents should initially be displayed. IncludeHeight will display the subComponent below each row (w/o expand/collapse button) and include its initial height for the container height calculation defined by visibleRows.

Supporting expandable subcomponents with dynamic table container height calculation would lead to "jumping" table container sizes, which is not how the table should behave. Also, nesting tables inside subcomponents is something we can't support because of a11y and UX concerns. Of course performance is also a factor here, we didn't analyze it yet, but I would assume that this behavior wouldn't have a great influence on performance, because we only can measure the subcomponent once mounted (on expand) and then would have to recalculate the whole virtualisation and container size. Last but not least, this feature is not supported by any SAP/OpenUI5 table.

[texttechne]

Hi @Lukas742,

Disabling Virtualization

Dynamically removing the logic of the virtualisation is not practical as its deeply implemented in the table core and cannot be controlled by hooks. Refactoring this would result in a huge effort on our end and even with excessive testing, we probably wouldn't catch all edge cases.

yeah, that was what I was experiencing when trying to create the PR 😅. So I absolutely agree with that: The AnalyticalTable uses virtualization at its core. This feature request goes against it, so it shouldn't be done.

For that extreme use case that I faced - 3 nested tables (some virtualized and some not) - I created a new table component based on the code of the AnalyticalTable and adapted to the specific use case. Actually the most complicated part was implementing checkboxes and the selection logic across this hierarchy...

if you want to disable virtualisation, please set the overscanCount or the overScanCountHorizontal to a very high number.

You effectively disable virtualization this way, but this doesn't solve the problem which is actually rooted in the height calculation. And this height calculation is required as soon as you employ virtualization. So not using virtualization is a prerequisite / enabler to not calculate the height dynamically....

However, I asked for disabling the virtualization because of my use case with the nested tables. One of the inner tables would have needed that feature in order to take up the height it had... so quite a special use case...

Respecting subcomponents in height calculation

Supporting expandable subcomponents with dynamic table container height calculation would lead to "jumping" table container sizes, which is not how the table should behave.

Actually, the sandbox example still looks and feels like a bug to me:

• take the behavior of the expandable example of the documentation
  • each subComponent takes up the space it needs as it is expanded
• compare it to the behavior of my sandbox example
  • expanding the subComponent looks ridiculous: it only has the the space of the 2 rows available
  • the only work-around is to set minRows to a large value to artificially take up the container height (there are plenty of cases where this is not acceptable)

The difference is that the sandbox example only consists of 2 rows and this determines the height of the container. My expectation is that the container of the table operates "responsive" between a min and a max height (by default defined by minRows and visibleRows, right?). So in the sandbox example it would grow with the subComponent up to the max height (= 15 visible rows).

I would be totally fine with setting the max height statically from outside if necessary.

[Lukas742]

Hi @texttechne

thank you for your feedback. We decided to offer a way to programmatically set the bodyHeight of the table, but not to document it as it could lead to strange behavior when used recklessly.

After this PR is merged and released, you will be able to set the bodyHeight of the table by updating the internal table state bodyHeight. You can for example achieve this with a custom stateReducer (plugin-hook) or by leveraging useControlledState (see here):

const TableComponent = (props) => {
  const [bodyHeight, setBodyHeight] = useState(undefined);
  const useControlledState = (state) => {
    return useMemo(() => {
      return { ...state, bodyHeight };
    }, [state, bodyHeight]);
  };
  return (
    <>
      <Button
        onClick={() => {
          setBodyHeight(800);
        }}
      >
        Set Body Height 800
      </Button>
      <AnalyticalTable {...props} reactTableOptions={{ useControlledState }} />
    </>
  );
};

[texttechne]

Hi @Lukas742,

ahh cool. Yeah, I think that's absolutely sufficient for that use case. Thanks!

I would disagree regarding the documentation: With this issue you've already documented this feature, it's only hard to find 😄. A big fat warning would do the trick, I think. I can also imagine to move the subcomponent part of the documentation to the very end of it (as it is a very special feature). And then add this special note and example there.

And then again, this is really only my 2 cents, so feel free to ignore this... just my gut feeling...

[gitgdako]

Hi. I might not have understood correctly what your reasoning here was but the workarounds are insufficient in our opinion, as the height calculation would have to be done by us. The case of a table with a single visible row as described in the above linked duplicate by myself can not be sensibly resolved. We have played around a bit and using InludeHeight created our own button per row that tracks which subcomponents are rendered and render empty Fragment for the others which actually gives "decent" results. The problem mentioned in the this ticket is way more complex than what we try to achieve (see duplicate), therefore we would like to either have this issue reopened and the simpler example addressed or do the same somewhere else. Thank you

[texttechne]

Hi @gitgdako,

yeah this ticket mixes two issues at once: Disabling virtualization and respecting the subcomponent height in the overall height calculation. You're confronted with the second one, but it's definitely a match with your issue.

as @Lukas742 pointed out:

After this https://github.com/SAP/ui5-webcomponents-react/pull/5223 is merged and released, you will be able to set the bodyHeight of the table by updating the internal table state bodyHeight. You can for example achieve this with a custom stateReducer (plugin-hook) or by leveraging useControlledState (see here

Have you already used this feature (doesn't seem to be released yet)? But yeah, boils down to you implementing it using what Lukas is proposing there...

I'm really divided here: On the one hand I'm on @gitgdako's side. The behavior of the Analytical Table in the mentioned use case just feels wrong, looks buggy and requires an own implementation of the height calculation. On the other hand it puts quite a burden on the Analytical Table as height calculation gets complicated and performance critical quite fast.... And then again, would you really want to force all users to implement something that complex?

Actually, it would be great to see how the mentioned workaround would look and feel like in real life. I mean, I usually don't know the height of any element I'm going to render beforehand... but maybe it's not that complicated as it seems right now...

[Lukas742]

Hi all,

first of all I have good news! We found a way to include expandable subcomponents in the height calculation of the table. (See PR)

Unfortunately this comes with a downside... Due to the structure of the table, we have to update the table every time a user expands/collapses a row, so it can lead to performance degradation. This was one of the reasons why we originally decided against implementing this behavior. However, since we've now found an acceptable way to implement it, we have decided to offer this feature because of your feedback.

Supporting expandable subcomponents with dynamic table container height calculation would lead to "jumping" table container sizes, which is not how the table should behave.

This is still something, that can severely affect UX, so you have to be careful when using subcomponents in this way (nobody likes jumping elements or double scollbars :wink:)

[github-actions[bot]]

:tada: This issue has been resolved in version v1.23.0 :tada:

The release is available on v1.23.0

Your semantic-release bot :package::rocket: