Announcement

Collapse
No announcement yet.
X
  • Filter
  • Time
Clear All
new posts

    SmartGWT 13.0: Record components of varying height rendered incorrectly

    Hi,

    We have a grid that renders content with varying heights, as record components. It seems that the record components are positioned all over the place and we are having trouble fixing the issue.

    The issue is reproducible with the following code:

    Code:
        private DataSource ds;
        private ListGrid grid;
    
        public void doOnModuleLoad() {
            viewport = new VLayout();
            viewport.setWidth100();
            viewport.setHeight100();
    
            ds = new DataSource();
            DataSourceTextField pk = new DataSourceTextField("id");
            pk.setPrimaryKey(true);
            DataSourceIntegerField sub = new DataSourceIntegerField("sub");
            ds.setFields(pk, sub);
            ds.setClientOnly(true);
    
            grid = new ListGrid() {
                @Override
                protected Canvas createRecordComponent(ListGridRecord r, Integer col) {
                    if(col == 1) {
                        int sub = r.getAttributeAsInt("sub");
    
                        if(sub > 0) {
                            // return a sub listgrid with randomized nr of rows in it
                            ListGrid g = new ListGrid();
    
                            g.setHeight(1);
                            g.setOverflow(Overflow.VISIBLE);
                            g.setBodyOverflow(Overflow.VISIBLE);
    
                            g.setFixedRecordHeights(false);
                            g.setWrapCells(true);
    
                            DataSource sds = new DataSource();
                            DataSourceTextField f1 = new DataSourceTextField("f1");
                            f1.setPrimaryKey(true);
                            DataSourceTextField f2 = new DataSourceTextField("f2");
                            sds.setFields(f1, f2);
                            sds.setClientOnly(true);
    
                            g.setDataSource(sds);
    
                            Record[] sd = new Record[sub];
                            for(int i=0; i<sub; i++) {
                                ListGridRecord s = new ListGridRecord();
                                s.setAttribute("f1", "pk" + i);
                                // randomize some wrapping text
                                s.setAttribute("f2", "Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s, when an unknown printer took a galley of type and scrambled it.".substring(0, Random.nextInt(215)));
                                sd[i] = s;
                            }
                            g.setData(sd);
    
                            return g;
                        }
                    }
                    return null;
                }
            };
            grid.setWidth100();
            grid.setHeight100();
    
            grid.setFixedRecordHeights(false);
    
            grid.setDataSource(ds);
            grid.setAutoFetchData(false);
    
            grid.setShowRecordComponents(true);
            grid.setShowRecordComponentsByCell(true);
            grid.setRecordComponentPosition(EmbeddedPosition.WITHIN);
            grid.setRecordComponentPoolingMode(RecordComponentPoolingMode.DATA);
    
            ListGridField pkField = new ListGridField("id");
            pkField.setWidth(80);
    
            ListGridField subField = new ListGridField("sub");
    
            grid.setFields(pkField, subField);
    
            viewport.addMember(grid);
    
            Button b = new Button("refresh");
            b.addClickHandler(clickEvent -> {
                refresh();
            });
            viewport.addMember(b);
    
            viewport.draw();
        }
    
        public void refresh() {
            ds.setTestData(new Record[] { gr(1), gr(2), gr(3), gr(4), gr(5), gr(6), gr(7), gr(8) });
            ds.fetchData(null, (response, rawData, request) -> {
                final ResultSet resultSet = new ResultSet();
                resultSet.setDataSource(ds);
                Record[] data = response.getData();
                resultSet.setAllRows(data);
                grid.setData(resultSet);
            });
        }
    
        public ListGridRecord gr(int i) {
            ListGridRecord r = new ListGridRecord();
            r.setAttribute("id", "primarykey" + i);
            r.setAttribute("sub", Math.abs(Random.nextInt(4)));
            return r;
        }
    Click "refresh" button at the bottom to randomize some content to the grid. The problem seems to lie in the fact that as the grid rendered as a record component is wrapping it's content and it has a varying height (overflow visible), the framework has trouble rendering the components in the correct position. The problem is always "fixed" if you resize your browser a bit to force a redraw.

    Is this a bug, is there something wrong with the code or is there a workaround?

    Using SmartClient Version: v13.0p_2022-01-30/LGPL Development Only (built 2022-01-30), latest Chrome browser on OS X.

    #2
    While you can provide recordComponents of varying heights, you can’t provide components that asynchronously resize, as if the case with your nested ListGrid (they resize after the asynchronous call to the DataSource is complete).

    We know this test case is synthetic, but the fix here would be to pre-fetch the related data, or at least know the counts in advance, which would allow the sub-grids height to be known in advance.

    Comment


      #3
      Ok, well why does the same actual code work with SmartGWT 12.1p?

      The actual problem seems to be that the record components are actually placed incorrectly - it looks like the rows are actually sized correctly. We adapted the test case a bit to render just three rows (first as red, second as green, and the third as blue):

      Click image for larger version

Name:	screenshot.png
Views:	130
Size:	211.6 KB
ID:	267285
      To my eye, it seems that the rows and the sub-components are of the correct size, but the components are just placed in the wrong position.

      Comment


        #4
        To add to this, yes, the test case is synthetic but it demonstrates a problem we encountered in our real-world app when upgrading from 12.1p to 13.0. This issue represents itself in various places but to give you one example, we have a view that renders a change history for some of our (app's) assets. This change history is fetched from the server and has a simple model as

        HistoryDS --* HistoryChangeDS

        History represents one "edit" of the asset and HistoryChange represents a changed attribute in the asset. For each edit of the asset, a History record is returned from the server and the record contains a list of HistoryChange records in it to represent all the attributes that changed in the edit of the asset. As UI for the user, we have a simple ListGrid which renders these History records. As record components, we have a grid that grabs the HistoryChange rows from the record and renders them to an embedded ListGrid.

        With 12.1p this worked but as we upgrade to 13.0, the grids for HistoryChange are drawn incorrectly, as demonstrated with this synthetic test case.

        Comment


          #5
          This is not a supported approach in any version - if it worked in 12.1, it would be just luck regarding the timing of the fetches and redraws of the embedded grids vs the main grid.

          Specifically, when you provide a recordComponent, we need to know its height so we can figure out which rows are in view given the scroll position, which has downstream implications regarding incremental rendering, data paging, etc. If components are allowed to change heights on the fly, we would have no choice but to just recalculate everything and redraw everything - potentially over and over again as different components resize - resulting in user-visible problems like the scroll position hopping all over the place, and not knowing how many rows we need to fetch from the server.

          So, the grid does handle varying heights for rows with incremental rendering and data paging - already a very hard problem that no other framework solves - but changing heights on the fly is not a tractable problem.

          So you will need to find a way that the height can be known when createRecordComponent returns, as previously suggested.

          Also, as far as sizing a grid to fit its data, you should be using autoFitData, not setting overflow on the grid and body.

          Comment


            #6
            Ok, thanks.

            To make this as clear as possible, what we are really targeting is basically a "simple HTML table" with no nested scroll bars present. A simple case. One cell of the table has some text of varying length that will sometimes wrap making the cell's height dynamic. This is the case that the browser rendering engine tackles and surely it has all the same challenges for fitting the data as you explain (your) grid renderer has.

            We surely understand the technical complexity under the hood but as a developer using the API, it would be cool to get a clear picture of what is really supported and what is not. We do not have knowledge of the inner workings of the framework and still, we are a bit baffled about this as we have been using this strategy for years.

            There is nothing in our code - at least intentionally - that will change the height of the nested grid dynamically. Let's make this perfectly clear. The main grid makes one fetch from the server and then renders everything. We don't see why the nested grids would be asynchronous as we provide all the records to them with setRecords(Record[]) call. They would of course be asynchronous and change height if we would call fetchData on them (or something in those lines) but we do not.

            We would appreciate it if you could answer the questions below. I'm pretty sure others will stumble on this thread so it would be great to get into the detail about this.

            1) A nested (record component) grid with setWrapCells(true) + setFixedRecordHeights(false) is only supported if it has explicit height?

            Now, if we've understood correctly, if we have a ListGrid that has a column that is allowed to wrap long text content, only way for it to work is to explicitly set a height on the grid. And have scrollbars visible for the user.

            2) "you will need to find a way that the height can be known when createRecordComponent returns"

            And if this is not possible, you are out of luck and there are no possible strategies - even if they would be a bit "dirty redraw everything afterward"?

            In our simple case, there is no way to know what is the height of the (wrapping) long text we are rendering to the cell.

            3) A ListGrid as illustrated in the attached image is not possible with the ListGrid API?

            Click image for larger version  Name:	wrapping_nested_grid.png Views:	0 Size:	26.9 KB ID:	267297

            Excuse us for our skills in Google sheets. The Changes-field has a nested ListGrid that will render simple key-value pairs. It is essential to show the content to the user. Everything will work if we set the nested grid to have fixed row heights but that is not acceptable in this use case.
            Last edited by markok; 3 Feb 2022, 23:09.

            Comment


              #7
              Hi markok,

              I'm using a similar thing for a "facebook messenger" style chat with different height bubbles in 12.0p. This is working fine as well.
              My bubbles are Labels and not inner ListGrids, but if you can draw the inner ListGrid synchronously, I don't see why this should not work. Old sample of my code is here.

              Differences to my usage compared to yours are:
              • Skin: Enterprise -> Tahoe based
              • RecordComponentPoolingMode: data -> recycle

              Best regards
              Blama

              Comment


                #8
                Hi,

                To be clear Blama , are you saying your code is working on 13.0? All our code works ok in 12 branch too.

                We have tried Enterprise and Graphite skins, it has no effect (not surprising). We also tried our synthetic test case by replacing the nested grid with simple

                Code:
                    Label l = new Label("a very long wrapping text here ......");
                    l.setWrap(true);
                    return l;
                And it is not working. The labels get drawn to incorrect positions too. That would be expected if it is really true that this is not supported.

                Comment


                  #9
                  Hi markok,

                  no, I'm on an old 12.0p. Did you also try if RecordComponentPoolingMode makes a difference? Perhaps just comment it out and go with the default of viewport, which is slower, but removes a chance of problem as there is no reuse.

                  Best regards
                  Blama

                  Comment


                    #10
                    Thanks for your input Blama . We made this as simple as we could and

                    Code:
                        private DataSource ds;
                        private ListGrid grid;
                    
                        public void doOnModuleLoad() {
                            viewport = new VLayout();
                            viewport.setWidth100();
                            viewport.setHeight100();
                    
                            ds = new DataSource();
                            DataSourceTextField pk = new DataSourceTextField("id");
                            pk.setPrimaryKey(true);
                            DataSourceIntegerField sub = new DataSourceIntegerField("sub");
                            ds.setFields(pk, sub);
                            ds.setClientOnly(true);
                    
                            grid = new ListGrid() {
                                @Override
                                protected Canvas createRecordComponent(ListGridRecord r, Integer col) {
                                    if(col == 1) {
                                        Label l = new Label(r.getAttribute("text"));
                                        l.setWrap(true);
                                        l.setBackgroundColor(r.getAttribute("color"));
                                        return l;
                                    }
                                    return null;
                                }
                            };
                            grid.setWidth100();
                            grid.setHeight100();
                            grid.setFixedRecordHeights(false);
                            grid.setDataSource(ds);
                            grid.setAutoFetchData(false);
                            grid.setShowRecordComponents(true);
                            grid.setShowRecordComponentsByCell(true);
                            grid.setRecordComponentPosition(EmbeddedPosition.WITHIN);
                            ListGridField pkField = new ListGridField("id");
                            pkField.setWidth(80);
                            ListGridField subField = new ListGridField("sub");
                            grid.setFields(pkField, subField);
                            viewport.addMember(grid);
                    
                            Button b = new Button("refresh");
                            b.addClickHandler(clickEvent -> {
                                refresh();
                            });
                            viewport.addMember(b);
                    
                            viewport.draw();
                        }
                    
                        public void refresh() {
                            // refresh grid with some made-up data
                            grid.setData(r(1), r(2), r(3));
                        }
                    
                        public ListGridRecord r(int i) {
                            ListGridRecord r = new ListGridRecord();
                            r.setAttribute("id", "primarykey" + i);
                            String text = ("Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has " +
                                    "been the industry's standard dummy text ever since the 1500s, when an unknown printer took a galley " +
                                    "been the industry's standard dummy text ever since the 1500s, when an unknown printer took a galley " +
                                    "been the industry's standard dummy text ever since the 1500s, when an unknown printer took a galley " +
                                    "of type and scrambled it.").substring(0, Random.nextInt(315));
                            r.setAttribute("text", text);
                            if(i == 1)
                                r.setAttribute("color", "#f00");
                            else if(i == 2)
                                r.setAttribute("color", "#0f0");
                            else if(i == 3)
                                r.setAttribute("color", "#00f");
                            return r;
                        }
                    .. And this happens:

                    Click image for larger version

Name:	labels.png
Views:	180
Size:	76.7 KB
ID:	267302
                    I don't understand what we could do to make the ListGrid "synchronized". To my eye, it seems that the problem is the wrapping content.

                    Comment


                      #11
                      Hi markok,

                      my gut feeling says that ListGrid.setData() from your sample is already synchronous, while fetchData() would not be. (I'm talking only about the inner ListGrids, the outer ListGrid obviously should be able to use paged fetches)
                      But if you can reproduce the issue with Labels instead of inner ListGrids as well, then this should be even easier for Isomophic to recreate.

                      Best regards
                      Blama
                      Last edited by Blama; 4 Feb 2022, 03:15.

                      Comment


                        #12
                        This is the case that the browser rendering engine tackles and surely it has all the same challenges for fitting the data as you explain (your) grid renderer has.
                        Just a note that no, the browser rendering engine does not have to tackle this problem at all. We are dealing with incremental loading and incremental rendering of a data set where we can't know all the heights in advance, and we need to make it scroll smoothly, no jumping. Browsers just render a fully-loaded page, and if they do any pre-rendering while resources are still loading, they feel free to bounce all over the place.

                        However, this more recent test case no longer has a problem with delayed loading or unsupported overflow settings, so we're checking it out.

                        Comment


                          #13
                          Originally posted by Isomorphic View Post
                          However, this more recent test case no longer has a problem with delayed loading or unsupported overflow settings, so we're checking it out.
                          Cool thanks. Just to ask - what is "delayed loading" in the original test case? Are you saying the grid.setRecords(Record[]) is asynchronous as it is or, is there something else in the original test case that makes the nested grids asynchronous?

                          Comment


                            #14
                            We've made a change to address the issue with overflow: "visible" recordComponents potentially rendering with the wrong top-offsets.

                            Please retest with a build dated February 9 or later.

                            Comment


                              #15
                              Please note that this fix will be in the February 9 build of 13.1, as we said, but the change didn't make it into the 13.0 build queue in time, so it will hit in that branch tomorrow, in builds dated February 10.

                              Comment

                              Working...
                              X