Ticket #1953 (closed defect: fixed)

Opened 7 weeks ago

Last modified 5 weeks ago

IGridlayout fails to render rows with colspanned rows properly

Reported by: Joonas Lehtinen Owned by: Artur Signell
Priority: blocker Milestone: User Interface Library 5.3.0 RC
Component: gwt-adapter-client Version: 5.2.5
Keywords: Cc:
Known Issue description:
Hours estimate: Deadline (dd.mm.yyyy):
Known Issue version (since): Known Issue title:
Hours done: Depends to:
Affects documentation: no
Known Issue workaround:
Affects release notes: no Contract:

Description (last modified by Matti Tahvonen) (diff)

Test case containing two grids that render badly in trunk.

Problem is that render function trusts that GWT Grid will return the right cell by index. But GWT Grids indexes on rows are actually just the index of TD in TR as a children, not a real grid cell index. Layout function must implement this kind of logic by itself.

See also forum post: http://forum.itmill.com/posts/list/449.page (the second grid in test case).

Attachments

Picture 2.png (71.2 kB) - added by Joonas Lehtinen 7 weeks ago.

Change History

Changed 7 weeks ago by Joonas Lehtinen

Changed 7 weeks ago by rokklimber

  • summary changed from GrinLayout generates unnecessary empty td:s after multicolumn cells to GridLayout generates unnecessary empty td:s after multicolumn cells

I traced the source of the problem to the interaction between IGridLayout and FlexTable?. When a new cell is created it calls prepareCell to ensure that the cell exists. FlexTable? counts the current number of cells in the row without taking their width into account and generates an extra cell for each column in between the last cell and the next one. In order to fix this a smarter cell formatter might need to be written that takes the width of the cell into account when creating new cells. In the short term these cells can be detected easily due to their lack of style settings. I added a filter to the IGridLayout class that can be turned on by the use of a style parameter that will filter out the bad cells from the row by removing any cells that have no style information. In order for this to work you must apply styles to GridLayout? objects like so:

GridLayout grid = new GridLayout(x,y);
grid.addStyleName("grid-layout-trim");

and then replace IGridLayout with the following code changes:

Code Changes for Workaround:

/* 

 * Copyright 2008 IT Mill Ltd.

 * 

 * Licensed under the Apache License, Version 2.0 (the "License"); you may not

 * use this file except in compliance with the License. You may obtain a copy of

 * the License at

 * 

 * http://www.apache.org/licenses/LICENSE-2.0

 * 

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT

 * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the

 * License for the specific language governing permissions and limitations under

 * the License.

 */



package com.itmill.toolkit.terminal.gwt.client.ui;



import java.util.ArrayList;

import java.util.HashMap;

import java.util.Iterator;



import com.google.gwt.user.client.Command;

import com.google.gwt.user.client.DOM;

import com.google.gwt.user.client.DeferredCommand;

import com.google.gwt.user.client.Element;

import com.google.gwt.user.client.ui.FlexTable;

import com.google.gwt.user.client.ui.HasHorizontalAlignment;

import com.google.gwt.user.client.ui.HasVerticalAlignment;

import com.google.gwt.user.client.ui.SimplePanel;

import com.google.gwt.user.client.ui.Widget;

import com.google.gwt.user.client.ui.FlexTable.FlexCellFormatter;

import com.google.gwt.user.client.ui.HTMLTable.CellFormatter;

import com.google.gwt.user.client.ui.HasHorizontalAlignment.HorizontalAlignmentConstant;

import com.google.gwt.user.client.ui.HasVerticalAlignment.VerticalAlignmentConstant;

import com.itmill.toolkit.terminal.gwt.client.ApplicationConnection;

import com.itmill.toolkit.terminal.gwt.client.BrowserInfo;

import com.itmill.toolkit.terminal.gwt.client.CaptionWrapper;

import com.itmill.toolkit.terminal.gwt.client.Container;

import com.itmill.toolkit.terminal.gwt.client.ContainerResizedListener;

import com.itmill.toolkit.terminal.gwt.client.Paintable;

import com.itmill.toolkit.terminal.gwt.client.StyleConstants;

import com.itmill.toolkit.terminal.gwt.client.UIDL;

import com.itmill.toolkit.terminal.gwt.client.Util;



public class IGridLayout extends SimplePanel implements Paintable, Container,

        ContainerResizedListener {



    public static final String CLASSNAME = "i-gridlayout";



    private Grid grid = new Grid();



    private boolean needsLayout = false;



    private boolean needsFF2Hack = BrowserInfo.get().isFF2();



    private Element margin = DOM.createDiv();



    private Element meterElement;



    private String width;

    

    private boolean applyTrimmer = false;



    public IGridLayout() {

        super();

        DOM.appendChild(getElement(), margin);

        DOM.setStyleAttribute(getElement(), "overflow", "hidden");

        setStyleName(CLASSNAME);

        setWidget(grid);

    }



    protected Element getContainerElement() {

        return margin;

    }



    public void setWidth(String width) {

        this.width = width;

        if (width != null && !width.equals("")) {

            needsLayout = true;

        } else {

            needsLayout = false;

            grid.setWidth("");

        }

    }



    public void updateFromUIDL(UIDL uidl, ApplicationConnection client) {



        if (client.updateComponent(this, uidl, false)) {

            return;

        }

        final MarginInfo margins = new MarginInfo(uidl

                .getIntAttribute("margins"));



        setStyleName(margin, CLASSNAME + "-" + StyleConstants.MARGIN_TOP,

                margins.hasTop());

        setStyleName(margin, CLASSNAME + "-" + StyleConstants.MARGIN_RIGHT,

                margins.hasRight());

        setStyleName(margin, CLASSNAME + "-" + StyleConstants.MARGIN_BOTTOM,

                margins.hasBottom());

        setStyleName(margin, CLASSNAME + "-" + StyleConstants.MARGIN_LEFT,

                margins.hasLeft());



        setStyleName(margin, CLASSNAME + "-" + "spacing", uidl

                .hasAttribute("spacing"));

        iLayout();

        

        if(getStyleName().indexOf("grid-layout-trim") >= 0)

        {

          applyTrimmer = true;

        }

        

        grid.updateFromUIDL(uidl, client);

    }



    public boolean hasChildComponent(Widget component) {

        return grid.hasChildComponent(component);

    }



    public void replaceChildComponent(Widget oldComponent, Widget newComponent) {

        grid.replaceChildComponent(oldComponent, newComponent);

    }



    public void updateCaption(Paintable component, UIDL uidl) {

        grid.updateCaption(component, uidl);

    }



    public class Grid extends FlexTable implements Paintable, Container {



        /** Widget to captionwrapper map */

        private final HashMap widgetToCaptionWrapper = new HashMap();



        public Grid() {

            super();

            setStyleName(CLASSNAME + "-grid");

        }



        public void updateFromUIDL(UIDL uidl, ApplicationConnection client) {



            int row = 0, column = 0;



            final ArrayList oldWidgetWrappers = new ArrayList();

            for (final Iterator iterator = iterator(); iterator.hasNext();) {

                oldWidgetWrappers.add(iterator.next());

            }

            clear();



            final int[] alignments = uidl.getIntArrayAttribute("alignments");

            int alignmentIndex = 0;



            for (final Iterator i = uidl.getChildIterator(); i.hasNext();) {

                final UIDL r = (UIDL) i.next();

                if ("gr".equals(r.getTag())) {

                    column = 0;

                    

                    for (final Iterator j = r.getChildIterator(); j.hasNext();) {

                        final UIDL c = (UIDL) j.next();

                        if ("gc".equals(c.getTag())) {

                            prepareCell(row, column);

                          

                            // Set cell width

                            int w;

                            if (c.hasAttribute("w")) {

                                w = c.getIntAttribute("w");

                            } else {

                                w = 1;

                            }



                            FlexCellFormatter formatter = (FlexCellFormatter) getCellFormatter();



                            // set col span

                            formatter.setColSpan(row, column, w);



                            String styleNames = CLASSNAME + "-cell";

                            if (column == 0) {

                                styleNames += " " + CLASSNAME + "-firstcol";

                            }

                            if (row == 0) {

                                styleNames += " " + CLASSNAME + "-firstrow";

                            }

                            formatter.setStyleName(row, column, styleNames);



                            // Set cell height

                            int h;

                            if (c.hasAttribute("h")) {

                                h = c.getIntAttribute("h");

                            } else {

                                h = 1;

                            }

                            ((FlexCellFormatter) getCellFormatter())

                                    .setRowSpan(row, column, h);



                            final UIDL u = c.getChildUIDL(0);

                            if (u != null) {



                                AlignmentInfo alignmentInfo = new AlignmentInfo(

                                        alignments[alignmentIndex++]);



                                VerticalAlignmentConstant va;

                                if (alignmentInfo.isBottom()) {

                                    va = HasVerticalAlignment.ALIGN_BOTTOM;

                                } else if (alignmentInfo.isTop()) {

                                    va = HasVerticalAlignment.ALIGN_TOP;

                                } else {

                                    va = HasVerticalAlignment.ALIGN_MIDDLE;

                                }



                                HorizontalAlignmentConstant ha;



                                if (alignmentInfo.isLeft()) {

                                    ha = HasHorizontalAlignment.ALIGN_LEFT;

                                } else if (alignmentInfo.isHorizontalCenter()) {

                                    ha = HasHorizontalAlignment.ALIGN_CENTER;

                                } else {

                                    ha = HasHorizontalAlignment.ALIGN_RIGHT;

                                }



                                formatter.setAlignment(row, column, ha, va);



                                final Paintable child = client.getPaintable(u);

                                CaptionWrapper wr;

                                if (widgetToCaptionWrapper.containsKey(child)) {

                                    wr = (CaptionWrapper) widgetToCaptionWrapper

                                            .get(child);

                                    oldWidgetWrappers.remove(wr);

                                } else {

                                    wr = new CaptionWrapper(child, client);

                                    widgetToCaptionWrapper.put(child, wr);

                                }



                                setWidget(row, column, wr);



                                DOM.setStyleAttribute(wr.getElement(),

                                        "textAlign", alignmentInfo

                                                .getHorizontalAlignment());



                                if (!u.getBooleanAttribute("cached")) {

                                    child.updateFromUIDL(u, client);

                                }

                            }

                            column += w;

                        }

                    }

                    if(applyTrimmer) trimCells(row);

                    row++;

                }

            }



            // loop oldWidgetWrappers that where not re-attached and unregister

            // them

            for (final Iterator it = oldWidgetWrappers.iterator(); it.hasNext();) {

                final CaptionWrapper w = (CaptionWrapper) it.next();

                client.unregisterPaintable(w.getPaintable());

                widgetToCaptionWrapper.remove(w.getPaintable());

            }

            // fix rendering bug on FF2 (#1838)

            if (needsFF2Hack) {

                DeferredCommand.addCommand(new Command() {

                    public void execute() {

                        Element firstcell = getCellFormatter().getElement(0, 0);

                        if (firstcell != null) {

                            String styleAttribute = DOM.getStyleAttribute(

                                    firstcell, "verticalAlign");

                            DOM.setStyleAttribute(firstcell, "verticalAlign",

                                    "");

                            int elementPropertyInt = DOM.getElementPropertyInt(

                                    firstcell, "offsetWidth");

                            DOM.setStyleAttribute(firstcell, "verticalAlign",

                                    styleAttribute);

                            if (elementPropertyInt > 0) {

                                needsFF2Hack = false;

                            }

                        }

                    }

                });

            }

        }



        public boolean hasChildComponent(Widget component) {

            if (widgetToCaptionWrapper.containsKey(component)) {

                return true;

            }

            return false;

        }



        public void replaceChildComponent(Widget oldComponent,

                Widget newComponent) {

            // TODO Auto-generated method stub



        }



        public void updateCaption(Paintable component, UIDL uidl) {

            final CaptionWrapper wrapper = (CaptionWrapper) widgetToCaptionWrapper

                    .get(component);

            wrapper.updateCaption(uidl);

        }

        

        // my internal trim method to remove obsolete cells

        private void trimCells(int row)

        {

          FlexCellFormatter formatter = (FlexCellFormatter) getCellFormatter();

          for(int i = 0;i < getCellCount(row);i++)

          {

            try

            {

              checkCellBounds(row, i);

            } catch (IndexOutOfBoundsException e)

            {

              continue;

            }

            String style = formatter.getStyleName(row, i);

            

            while(style.length() < 1)

            {

              removeCell(row, i);

              style = formatter.getStyleName(row, i);

            }

          }

        }



    }



    public void iLayout() {

        if (needsLayout) {

            super.setWidth(width);

            if (meterElement == null) {

                meterElement = DOM.createDiv();

                DOM.setStyleAttribute(meterElement, "overflow", "hidden");

                DOM.setStyleAttribute(meterElement, "height", "0");

                DOM.appendChild(getContainerElement(), meterElement);

            }

            int contentWidth = DOM.getElementPropertyInt(meterElement,

                    "offsetWidth");

            int offsetWidth = getOffsetWidth();



            grid.setWidth((offsetWidth - (offsetWidth - contentWidth)) + "px");

        } else {

            grid.setWidth("");

        }

        Util.runDescendentsLayout(this);

    }



}


Changed 7 weeks ago by Joonas Lehtinen

Thanks!

Marc is doing some code review and changes for IGridLayout in #1878 - it might be wise to resolve these issues at the same time.

Changed 5 weeks ago by Matti Tahvonen

  • owner changed from Marko Gronroos to Artur Signell
  • status changed from new to assigned

Changed 5 weeks ago by Matti Tahvonen

  • description modified (diff)
  • summary changed from GridLayout generates unnecessary empty td:s after multicolumn cells to IGridlayout fails to render rows with colspanned rows properly

Hi rokklimber!

Your problem is actually a symptom of a major problem that has been known, but buried below other issues. Your fix works works only for this particular case, so we really need a general fix here. I'm assigning this ticket for Artur as a top priority (our new talent in Toolkit team). We hope to get this fixed soon!

matti

Changed 5 weeks ago by Artur Signell

  • status changed from assigned to closed
  • resolution set to fixed

Fixed in changeset [5155]

Note: See TracTickets for help on using tickets.