SAP / openui5

OpenUI5 lets you build enterprise-ready web applications, responsive to all devices, running on almost any browser of your choice.
http://openui5.org
Apache License 2.0
2.94k stars 1.23k forks source link

[FIX] - Re-enables the recalculation of the scrollbar in the sap.ui.table.Table with lazy loading #3980

Closed pp-zub closed 6 months ago

pp-zub commented 6 months ago

In the InternalFix: https://github.com/SAP/openui5/commit/7b2ab6619e2e6ac4720fbc3f2ab325283da268ac#diff-b63b432627eae50ca8bc3bac2f1ecf68dc7b20f11c75f46d9d2e89a759f4f180

the function call "this._adjustToTotalRowCount();" has been removed from "Table.prototype._getRowContexts", but it is necessary to recalculate the scrollbar in the sap.ui.table.Table with an odata binding and lazy loading.

cla-assistant[bot] commented 6 months ago

CLA assistant check
All committers have signed the CLA.

boghyon commented 6 months ago

it is necessary to recalculate the scrollbar in the sap.ui.table.Table with an odata binding and lazy loading

Could you please elaborate more on the issue and share a minimal sample that fails without the _adjustToTotalRowCount call?

pp-zub commented 6 months ago

I'm using the Table with a OdataV4 Binding. When scrolling down, new data wasn't loaded and the Scrollbar reached the bottom, without even showing all the preloaded data.

As it reached the bottom, new Data wasn't fetched either. I checked for which runtimes it was working and since which it was broken and found the commit I mentioned.

I tried to provide a minimal sample in the first place, but the public OData Services don't provide datasets which have enough data to use the threshold and lazy load by scrolling.

I can prepare a minimal sample with a placeholder url which has to be replaced if that helps.
(It’s basically a GridTable with no settings and a row binding to an odatav4)

boghyon commented 6 months ago

I can prepare a minimal sample with a placeholder url which has to be replaced if that helps.

That would be great. Thanks!

pp-zub commented 6 months ago

https://codepen.io/Patrick-D3V/pen/MWxNvaq

I have set the variables above with a local Odata v4 service, a data source and the ident column

In the console I see a $skip=0&$top=110 query and the data is in the query

But when I scroll down in the table, it stops after 22 rows and the scroll bar is at the bottom and I can't scroll down any further

I can resize the window so that the scrollbar is recalculated based on the resizing of the window and scroll down to the bottom one more time I can also click in the cell and press the down arrow. The data is there, only the calculation of the height of the scrollbar is not correct.

I used the "https://openui5.hana.ondemand.com/resources/sap-ui-core.js" endpoint as the CDN here.

JumpNRun commented 6 months ago

The table requires to know the count for certain functionalities to work properly, this is also documented: https://ui5.sap.com/#/api/sap.ui.table.Table%23methods/getRows

For OData V4, the relevant parameter in the binding info is "$count": https://ui5.sap.com/#/api/sap.ui.model.odata.v4.ODataModel%23methods/bindList Adding this parameter should solve the issue.

Since the count is required in general, there is not enough test coverage for scenarios without count to validate this change with all possible combinations of models and table configurations. Before the internal fix that you mentioned, _adjustToTotalRowCount was not always called in _getRowContexts, but could have been skipped.

In my opinion, it's not worth supporting a case that shouldn't happen.