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

no scrollbar visible if setFixedBodyHeight too big #906

Closed
JuliusRuppert opened this issue Jan 8, 2024 · 10 comments
Closed

no scrollbar visible if setFixedBodyHeight too big #906

JuliusRuppert opened this issue Jan 8, 2024 · 10 comments
Assignees
Labels
enhancement New feature or request version 2.x.x Version 2.x.x issues
Milestone

Comments

@JuliusRuppert
Copy link

If the bodyheight of a table, set with the setFixedBodyHeight() method, is greater than the pixel amount of the initially loaded rows defined in the LocalListScrollingDataSource constructor, no scrollbar will appear. See JuliusRuppert/domino-ui-demo@4b2bff1.
Since the bodyheight is defined in px and the initially loaded rows are defined as the row amount, you have to do some calculations based on internal information of the table, e.g. the row height. These calculations have to ensure that the pixel amount of the initally loaded rows + table header etc. are greater than the pixel amount defined in setFixedBodyHeight().

So my suggestion would be to have a setFixedBodyHeigt() method which will accept the amount of rows or introducing a method that ensures that the scrollbar is visible by adjusting the BodyHeight to the possible maximum.

@vegegoku
Copy link
Member

Well, as a start the height of a row is dynamic and is not fixed by table or even for all rows in the same table, this is completely based on how you render the rows, and there is no way to do the calculation unless all rows are rendered, and this will have to be applied after any kind of update to the table data or when ever any kind of a resize is applied to the table or any of its elements.

also a calculation of such thing is expensive in terms of performance, and will result in a very clunky behaviors.

and last I dont see the reason why I would show scrollbars when the body is already bigger than the actual rendered rows, it does not make sense and even if i want to do that I would need to use filler to make the browser add the scrollbars, or we can use overflow:scroll and have disabled scrollbars.

I am not sure if I understand the issue correctly but I guess there might be more explanation for this issue.

@JuliusRuppert
Copy link
Author

Thank you very much for the detailed explanation. I was not fully aware of how the line height is calculated. Now I understand the problem better.

We noticed the problem with the missing scrollbars because we would like to adapt the table size to the current screen area. We set this size with setFixedBodyHeight() and the calc function of css. But when the tables body is too big, then the scrollbars will not appear and the user cannot load the missing rows.
So our problem is more about the usability than of cosmetic reasons.

Do you have any suggestions how we can solve this?

@FrankHossfeld
Copy link
Contributor

Your request sounds like the scrollbar is used as a "load more data" - button. In this case I would add a load more icon/button.

@vegegoku
Copy link
Member

Thank you very much for the detailed explanation. I was not fully aware of how the line height is calculated. Now I understand the problem better.

We noticed the problem with the missing scrollbars because we would like to adapt the table size to the current screen area. We set this size with setFixedBodyHeight() and the calc function of css. But when the tables body is too big, then the scrollbars will not appear and the user cannot load the missing rows. So our problem is more about the usability than of cosmetic reasons.

Do you have any suggestions how we can solve this?

The part that confuses me is :

is greater than the pixel amount of the initially loaded rows defined in the LocalListScrollingDataSource constructor, no scrollbar will appear.

So why there is a scroll bar if the body is already bigger, 2nd if you intend to use the LocalListScrollingDataSource you will need to make sure that the initial data loaded will exceed the available space, use a threshold that will always result in showing scrollbars.

If you cant have such insurance then you either need a different datasource or you need to add something to help you load more items, for example if you know you still have more records and there is no scrollbars you manually fire the BodyScrollEvent since it is a logical event from the data table point of view.

Maybe I am wrong, a reproducible or some screenshots might help understanding the issue.

@bschmelcher
Copy link

Hi @vegegoku,

So why there is a scroll bar if the body is already bigger, 2nd if you intend to use the LocalListScrollingDataSource you will need to make sure that the initial data loaded will exceed the available space, use a threshold that will always result in showing scrollbars.

We want to set a fixedBodyHeight for the TableBody based on the available screen space, so the table will take up all unused space of the page to minimize unused "white"-space of the page for tables with lots of entries.
Since the bodyHeight is a pixel-value, but the pageSize for the LocalListScrollingDataSource is a number of rows, we have to do some workaround-calculations to configure the LocalListScrollingDataSource so that always a scrollbar will be visible.
And since as you mentioned the rowHeight is dynamic, doing these calculations could be error-prone.

Another point here is that it would be useful if the number of entries that will be loaded upon scrolling should not be dependent on the number of entries initially loaded - as a longer rendering time upon initial loading of the table is acceptable, but rendering during scrolling should be fast - so it might make sense to set the number of entries loaded upon scrolling to be smaller than the initially loaded entries.
If we configure the LocalListScrollingDataSource's pageSize to something smaller than the number of entries needed to fill the fixedBodyHeight, no scrollbar will be shown - and we haven't found a solution yet how to check if a scrollbar is visible for the tableBody to manually trigger the BodyScrollEvent until a scrollbar is visible - we will need to investigate this more.

From our point of view, an optimal solution would load as much entries for scrollable tables until
a) a scrollbar is visible - independent of the number of rows configured for the LocalListScrollingDataSource.
Or b) all entries are shown in the table if the tableSize with all loaded entries set for the data of LocalListScrollingDataSource is still smaller than the fixedBodyHeight

Another question we have for the ScrollLoading: It would be awesome if the tableConfig would not have to be set to fixed for the ScrollLoading to work. Would it somehow be possible to decouple ScrollLoading from the fixed tableConfig?
We use an unfixed Table for our application as tables look much better when they are unfixed - as columns are distributed evenly across the available space without having to set manual widths.
So if we enable ScrollLoading, we can't profit from that really good feature anymore.
Should we open a separate issue for this?

See attached code for a prototype of our problems.

Code example
import com.google.gwt.core.client.EntryPoint;
import elemental2.dom.Event;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import org.dominokit.domino.ui.cards.Card;
import org.dominokit.domino.ui.cards.HeaderAction;
import org.dominokit.domino.ui.datatable.ColumnConfig;
import org.dominokit.domino.ui.datatable.DataTable;
import org.dominokit.domino.ui.datatable.TableConfig;
import org.dominokit.domino.ui.datatable.events.BodyScrollEvent;
import org.dominokit.domino.ui.datatable.plugins.BodyScrollPlugin;
import org.dominokit.domino.ui.datatable.plugins.BodyScrollPlugin.ScrollPosition;
import org.dominokit.domino.ui.datatable.plugins.SortPlugin;
import org.dominokit.domino.ui.datatable.plugins.SortPluginConfig;
import org.dominokit.domino.ui.datatable.store.LocalListScrollingDataSource;
import org.dominokit.domino.ui.icons.Icons;
import org.dominokit.domino.ui.layout.Layout;
import org.dominokit.domino.ui.style.ColorScheme;
import org.dominokit.domino.ui.utils.TextNode;

public class ScrollLoadingIssue implements EntryPoint {

    private LocalListScrollingDataSource<DataObject> localListScrollingDataSource;

    private DataTable<DataObject> table;

    private final TableConfig<DataObject> tableConfig = new TableConfig<>();

    private final Card card = Card.create();

    private final static int NUMBER_OF_COLUMNS = 5;

    private final static int NUMBER_OF_ROWS = 30;

    private final static int SCROLL_LOADING_ROW_NUMBER = 5;

    public static int reloadCounter = 0;

    @Override
    public void onModuleLoad() {

        Layout layout = Layout.create("Scroll Loading").show(ColorScheme.AMBER);

        initTableConfig();

        // We would like the loaded entries for scroll loading to be independent from the initial loaded entries.
        // Initial loading may take more time than loading additional rows on demand to ensure smooth loading upon scrolling.
        localListScrollingDataSource = new LocalListScrollingDataSource<>(SCROLL_LOADING_ROW_NUMBER);

        table = new DataTable<>(tableConfig, localListScrollingDataSource).noStripes();

        List<DataObject> data = createTableData(0);
        localListScrollingDataSource.setData(data);
        localListScrollingDataSource.load();

        card.addHeaderAction(new HeaderAction(Icons.MDI_ICONS.database_refresh_mdi()).addClickListener((Event evt) -> {
            table.fireTableEvent(new BodyScrollEvent(ScrollPosition.BOTTOM));
        }));

        // Suggested fix -> How should we check if the body of the table has a scrollbar?
//         table.fireTableEvent(new BodyScrollEvent(ScrollPosition.BOTTOM));
        card.appendChild(table);

        layout.getContentPanel().appendChild(card);
    }

    private void initTableConfig() {
        addTableConfigColumns();
        tableConfig.setFixedBodyHeight("400px");
        tableConfig.addPlugin(new BodyScrollPlugin<>());
        //It would be great if Scroll Loading also works with a not fixed TableConfig
        tableConfig.setFixed(true);
        tableConfig.addPlugin(new SortPlugin<DataObject>()
                .configure((SortPluginConfig config) -> {
                    config.setShowIconOnSortedColumnOnly(true);
                }));

    }

    private void addTableConfigColumns() {
        tableConfig
                .addColumn(ColumnConfig.<DataObject>create("index", "index")
                        .setCellRenderer(
                                cell -> TextNode.of(cell.getTableRow().getRecord().getValue("index").toString())
                        )
                        .sortable());

        for (int i = 1; i <= NUMBER_OF_COLUMNS; i++) {
            final int j = i;
            tableConfig.addColumn(ColumnConfig.<DataObject>create("column" + i, "column" + i)
                    .setCellRenderer(
                            cell -> TextNode.of(cell.getTableRow().getRecord().getValue("column" + j).toString()))
                    .sortable()
            );
        }
    }

    private List<DataObject> createTableData(int offset) {

        List<DataObject> data = new ArrayList<>();

        for (int i = 0; i <= NUMBER_OF_ROWS; i++) {
            DataObject object = new DataObject()
                    .addDataObject("index", (i + offset));
            for (int j = 1; j <= NUMBER_OF_COLUMNS; j++) {
                object.addDataObject("column" + j, "Value " + (i + offset));
            }
            data.add(object);
        }
        return data;
    }

    private static class DataObject {

        private final HashMap<String, Object> keyValuePair = new HashMap<>();

        public DataObject addDataObject(String key, Object value) {
            keyValuePair.put(key, value);
            return this;
        }

        public Object getValue(String key) {
            return keyValuePair.get(key);
        }
    }
}

@bschmelcher
Copy link

bschmelcher commented Jan 15, 2024

Hi @FrankHossfeld,

Your request sounds like the scrollbar is used as a "load more data" - button. In this case I would add a load more icon/button.

We think that scrollable tables is actually such a "load more data" feature, just without the need for users to switch between scrolling in the table and clicking the button - so with a greatly improved user experience.
Please correct us if we are wrong.

@FrankHossfeld
Copy link
Contributor

DataTable behave like everything inside the web. A scroll bar is only visible when there is not enough space to show the content. AFAIK even in Sencha GXT the scroll bar appears only in case the weights of the rows is greater than the available height. The only difference to Domino-UI is, that you can set a flag that add a 20 px wide container on the right, so that the columns are not redrawn once a scroll bar appears .

Regarding your request, I would say, that this is an unusual behavior for a web application. And I am not sure, that - pressing the scroll down button or clicking the slider - will create an event which can be caught and processed. And then, how many rows will be loaded and add to the store? one, ten, hundred? Also, I think, this will produce a lot of traffic ...

I would suggest using a pagination plugin.

@vegegoku vegegoku self-assigned this Jan 17, 2024
@vegegoku vegegoku added the enhancement New feature or request label Jan 17, 2024
@vegegoku vegegoku added the version 2.x.x Version 2.x.x issues label Jan 17, 2024
@vegegoku vegegoku added this to the 2.0.0 milestone Jan 17, 2024
@vegegoku
Copy link
Member

Another question we have for the ScrollLoading: It would be awesome if the tableConfig would not have to be set to fixed for the ScrollLoading to work. Would it somehow be possible to decouple ScrollLoading from the fixed tableConfig?
We use an unfixed Table for our application as tables look much better when they are unfixed - as columns are distributed evenly across the available space without having to set manual widths.
So if we enable ScrollLoading, we can't profit from that really good feature anymore.
Should we open a separate issue for this?

This is more complex than how it looks, I prefer this to be in its own issue and discussion.

@vegegoku
Copy link
Member

A new optional parameter added to the constructor of the LocalListScrollingDataStore that define the number of pages to be loaded in the initial load, for example new LocalListScrollingDataSource<Contact>(10, 3) will load 3o items in the first load and then while scrolling will load 10 at a time.

@JuliusRuppert
Copy link
Author

Hi @vegegoku,
thank you for this nice improvement.
To solve the missing scrollbar problem, we have implemented a functionality where we fire the BodyScrollEvent until a scrollbar is visible.
Maybe this approach does only work for our tested use cases. We are looking forward to your opinion.
If you find this improvement useful, it would be awesome if you could implement it (if possible for v1 as well) into domino-ui, so everybody can profit from this change.

See attached code

Code Example
import com.google.gwt.core.client.EntryPoint;
import elemental2.dom.Event;
import elemental2.dom.HTMLDivElement;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import org.dominokit.domino.ui.cards.Card;
import org.dominokit.domino.ui.cards.HeaderAction;
import org.dominokit.domino.ui.datatable.ColumnConfig;
import org.dominokit.domino.ui.datatable.DataTable;
import org.dominokit.domino.ui.datatable.TableConfig;
import org.dominokit.domino.ui.datatable.events.BodyScrollEvent;
import org.dominokit.domino.ui.datatable.plugins.BodyScrollPlugin;
import org.dominokit.domino.ui.datatable.plugins.BodyScrollPlugin.ScrollPosition;
import org.dominokit.domino.ui.datatable.plugins.BodyScrollPluginConfig;
import org.dominokit.domino.ui.datatable.plugins.SortPlugin;
import org.dominokit.domino.ui.datatable.plugins.SortPluginConfig;
import org.dominokit.domino.ui.datatable.store.LocalListScrollingDataSource;
import org.dominokit.domino.ui.icons.Icons;
import org.dominokit.domino.ui.layout.Layout;
import org.dominokit.domino.ui.style.ColorScheme;
import org.dominokit.domino.ui.utils.TextNode;

public class Poc implements EntryPoint {

    private LocalListScrollingDataSource<DataObject> localListScrollingDataSource;

    private DataTable<DataObject> table;

    private final TableConfig<DataObject> tableConfig = new TableConfig<>();

    private final Card card = Card.create();

    private final static int NUMBER_OF_COLUMNS = 5;

    private final static int NUMBER_OF_ROWS = 30;

    private final static int SCROLL_LOADING_ROW_NUMBER = 5;
    
    public static native boolean isScrollbarVisible(HTMLDivElement div) /*-{            
        var divHeight = div.getBoundingClientRect().height; 
        var tableheight = div.getElementsByTagName('thead')[0].getBoundingClientRect().height + div.getElementsByTagName('tbody')[0].getBoundingClientRect().height
            
        return isScrollbarVisible = tableheight > divHeight; 
    }-*/;
   
    @Override
    public void onModuleLoad() {

        Layout layout = Layout.create("Scroll Loading").show(ColorScheme.AMBER);

        initTableConfig();

        localListScrollingDataSource = new LocalListScrollingDataSource<>(SCROLL_LOADING_ROW_NUMBER);

        table = new DataTable<>(tableConfig, localListScrollingDataSource).noStripes();

        List<DataObject> data = createTableData(0);
        localListScrollingDataSource.setData(data);
        localListScrollingDataSource.load();

        card.addHeaderAction(new HeaderAction(Icons.MDI_ICONS.database_refresh_mdi()).addClickListener((Event evt) -> {
            table.fireTableEvent(new BodyScrollEvent(ScrollPosition.BOTTOM));
        }));
        
        card.appendChild(table);

        layout.getContentPanel().appendChild(card);
        
        while (!isScrollbarVisible(table.element())){
            table.fireTableEvent(new BodyScrollEvent(ScrollPosition.BOTTOM)); 
        }
    }

    private void initTableConfig() {
        addTableConfigColumns();
        tableConfig.setFixedBodyHeight("400px");
        tableConfig.addPlugin(new BodyScrollPlugin<DataObject>().setConfig(new BodyScrollPluginConfig(10)));
        //It would be great if Scroll Loading would work with fixed height only, so the columns span evenly over the available widht
        tableConfig.setFixed(true);
        tableConfig.addPlugin(new SortPlugin<DataObject>()
                .configure((SortPluginConfig config) -> {
                    config.setShowIconOnSortedColumnOnly(true);
                }));

    }

    private void addTableConfigColumns() {
        tableConfig
                .addColumn(ColumnConfig.<DataObject>create("index", "index")
                        .setCellRenderer(
                                cell -> TextNode.of(cell.getTableRow().getRecord().getValue("index").toString())
                        )
                        .sortable());

        for (int i = 1; i <= NUMBER_OF_COLUMNS; i++) {
            final int j = i;
            tableConfig.addColumn(ColumnConfig.<DataObject>create("column" + i, "column" + i)
                    .setCellRenderer(
                            cell -> TextNode.of(cell.getTableRow().getRecord().getValue("column" + j).toString()))
                    .sortable()
            );
        }
    }

    private List<DataObject> createTableData(int offset) {

        List<DataObject> data = new ArrayList<>();

        for (int i = 0; i <= NUMBER_OF_ROWS; i++) {
            DataObject object = new DataObject()
                    .addDataObject("index", (i + offset));
            for (int j = 1; j <= NUMBER_OF_COLUMNS; j++) {
                object.addDataObject("column" + j, "Value " + (i + offset));
            }
            data.add(object);
        }
        return data;
    }

    private static class DataObject {

        private final HashMap<String, Object> keyValuePair = new HashMap<>();

        public DataObject addDataObject(String key, Object value) {
            keyValuePair.put(key, value);
            return this;
        }

        public Object getValue(String key) {
            return keyValuePair.get(key);
        }
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request version 2.x.x Version 2.x.x issues
Projects
Status: Done
Development

No branches or pull requests

4 participants