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

using SetColSpan/SetRowSpan produce unnalligned tables #11

Closed
djadala opened this issue Mar 1, 2016 · 14 comments
Closed

using SetColSpan/SetRowSpan produce unnalligned tables #11

djadala opened this issue Mar 1, 2016 · 14 comments

Comments

@djadala
Copy link

djadala commented Mar 1, 2016

Hi,

in example 'Gowut - Showcase of Features'/'Advanced table structure with modified alignment, row and col spans' (see table container)
I expect cells 22 and 41 to miss and example table to be rectangular.
If i not set content for cells 2,2 & 4,1 gowut still produce empty <td></td>

@djadala
Copy link
Author

djadala commented Mar 1, 2016

Hi,
i have very simple & dirty solution,

add new component type:

type spanImpl struct  {
    compImpl    // Component implementation
}

func NewSpan() Comp  {
    return &spanImpl{ newCompImpl(nil) }
}

func (c *spanImpl) Render(w writer) {
    panic("must not be called")
}

and check in tableImpl Render:

func (c *tableImpl) Render(w writer) {
    w.Write(_STR_TABLE_OP)
    c.renderAttrsAndStyle(w)
    c.renderEHandlers(w)
    w.Write(_STR_GT)

    // Create a reusable cell index
    ci := cellIdx{}

    for row, rowComps := range c.comps {
        c.renderRowTr(row, w)
        for col, c2 := range rowComps {
            if _, ok := c2.(*spanImpl); !ok {
                ci.row, ci.col = row, col
                c.renderTd(ci, w)
                if c2 != nil {
                    c2.Render(w)
                }
            }
        }
    }

    w.Write(_STR_TABLE_CL)
}

and manually insert this type in cells you dont want to render:

t.Add(gwu.NewSpan(), 0, 2)

what you mind ?

Regards.
Jamil Djadala

@icza
Copy link
Owner

icza commented Mar 1, 2016

Will check your suggestion later when I have a chance.

@icza
Copy link
Owner

icza commented Mar 2, 2016

I'm not sure what you feel is wrong. I think the current behavior is correct. It follows the HTML table behavior.

Since all rows have 5 cells (5 values), and in the 3rd row (row idx=2) there is one cell which spawns to 2 columns, then the 3rd row will "reach beyond" the 2nd (will have an "extra" column over the 2nd for example).

@djadala
Copy link
Author

djadala commented Mar 2, 2016

may be not wrong, but i want rectangular table.
when i construct html table manually, i skip spanned cells, but gwu always produce them.
i just want to be able to skip some table cells.

@icza
Copy link
Owner

icza commented Mar 2, 2016

Ok, let's see an example. Using gwu.Table:

t := gwu.NewTable()
t.Add(gwu.NewLabel("00"), 0, 0)
t.Add(gwu.NewLabel("01"), 0, 1)
t.Add(gwu.NewLabel("02"), 0, 2)
t.Add(gwu.NewLabel("10-11"), 1, 0)
t.SetColSpan(1, 0, 2)
t.Add(gwu.NewLabel("12"), 1, 1)

The 2nd row contains 2 cells, first having colspan=2, so the 2nd cell in the 2nd row has index 1. This results in a rectangular table, no "empty" cells.

The "same" (same in structure) table generated with HTML:

ht := gwu.NewHtml(`<table><tr><td>00<td>01<td>02<tr><td colspan=2>10-11<td>12</table>`)

The raw HTML code formatted:

<table>
    <tr><td>00<td>01<td>02
    <tr><td colspan=2>10-11<td>12
</table>

I still think this is correct. Please explain in this example what you think is wrong.

@djadala
Copy link
Author

djadala commented Mar 2, 2016

but this sequence:

t.Add(gwu.NewLabel("00-01"), 0, 0)
t.SetColSpan(0,0,2)
t.Add(gwu.NewLabel("02"), 0, 1)

t.Add(gwu.NewLabel("10"), 1, 0)
t.Add(gwu.NewLabel("11"), 1, 1)
t.Add(gwu.NewLabel("12"), 1, 2)

doesn't work because Add call internally EnsureSize

@icza
Copy link
Owner

icza commented Mar 2, 2016

Have you tried it? Because your example code does work!

EnsureSize() is called to ensure the internal data structures of Table can accomodate the cell being added. It has nothing to do with cell layout. Your code does work as intended. It is structurally equvivalent to this HTML table:

<table>
    <tr><td colspan=2>00-01<td>02
    <tr><td>10<td>11<td>12
</table>

@djadala
Copy link
Author

djadala commented Mar 2, 2016

yes, i tried it. inspecting source in browser is:

<table id="68" cellspacing="0" cellpadding="0" class="gwu-Table">
    <tbody>
        <tr>
            <td colspan="2"><span id="69" class="gwu-Label">00-01</span></td>
            <td><span id="70" class="gwu-Label">02</span></td>
            <td></td>
        </tr>
        <tr>
            <td><span id="71" class="gwu-Label">10</span></td>
            <td><span id="72" class="gwu-Label">11</span></td>
            <td><span id="73" class="gwu-Label">12</span></td>
        </tr>
    </tbody>
</table>

@djadala
Copy link
Author

djadala commented Mar 2, 2016

to see visually extra cell, please try:

    t := gwu.NewTable()
    t.SetCellSpacing(3)
    t.SetCellPadding(30)

    t.Add(gwu.NewLabel("00-01"), 0, 0)
    t.SetColSpan(0,0,2)
    t.Add(gwu.NewLabel("02"), 0, 1)

    t.Add(gwu.NewLabel("10"), 1, 0)
    t.Add(gwu.NewLabel("11"), 1, 1)
    t.Add(gwu.NewLabel("12"), 1, 2)


    t.RowFmt(0).Style().SetBackground("#bbaa99")
    t.RowFmt(1).Style().SetBackground("#bbaa99")

@djadala
Copy link
Author

djadala commented Mar 2, 2016

ok, imho
in table.go, func (c *tableImpl) Add(c2 Comp, row, col int) bool {

instead of

    if row >= len(c.comps) || col >= len(c.comps[row]) {
        c.EnsureSize(row+1, col+1)
    }

should be

if row >= len(c.comps) {
    c.ensureRows(row+1)
}
if col >= len(c.comps[row]) {
    c.EnsureCols(row,col+1)
}

@icza
Copy link
Owner

icza commented Mar 2, 2016

Ok, now I see your concern, which is valid.

And you're proposed fix looks good and elegant!

@icza
Copy link
Owner

icza commented Mar 2, 2016

But actually I'm gonna implement a Table.Trim() method which will remove all trailing cells which have nil value. It will have a more general use.

@icza
Copy link
Owner

icza commented Mar 2, 2016

Thanks for reporting this, it is now fixed on gowut.dev:

icza/gowut.dev@13ed9ee

@icza icza closed this as completed Mar 2, 2016
@djadala
Copy link
Author

djadala commented Mar 2, 2016

I also thank,
Regards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants