set column width to new, narrower, value?

[lcplben]

Hello folks --

Problem page: www.sellmycalls.com/option-chain-ex.html
Problem code: lines 344-389

Each column TH has a minimize icon on the left. It's meant to minimize
the entire column by shrinking its width to some small number, like
10px or .5em.

After I click the icon, however, the column width/offsetWidth/
clientWidth doesn't change; it remains stuck at 46 on my machine.

What I've tried:

1. Save the contents of TDs on minimize, pop data on column restore:
works great but it's too easy to make column data be out of sync.
2. td.style.display = 'none' the data cells: disappears the
entire column of data cells, shifting all other column data cells left
and out from under their THs.
3. td.style.visibility = 'collapse' the data cells: sort of works, but
not completely.
4. td.style.overflow = 'hidden' the data cells: seems to me this
should definitely work, but it doesn't.

How can I set the width of the column to, say, .5em? (Naturally, I'm
really trying to set the width to that of a restore icon, but it seems
simpler to ignore that.)

Thanks!

-- ben

 

[Lasse Reichstein Nielsen]

Problem page: www.sellmycalls.com/option-chain-ex.html
Problem code: lines 344-389

Each column TH has a minimize icon on the left. It's meant to minimize
the entire column by shrinking its width to some small number, like
10px or .5em.

After I click the icon, however, the column width/offsetWidth/
clientWidth doesn't change; it remains stuck at 46 on my machine.

What I've tried:

1. Save the contents of TDs on minimize, pop data on column restore:
works great but it's too easy to make column data be out of sync.

You shouldn't keep your data in the table. For a web application, HTML
is presentation. Keep it in a data structure and rebuild the table
when necessary. That way the data can't get out of sync.

2. td.style.display = 'none' the data cells: disappears the
entire column of data cells, shifting all other column data cells left
and out from under their THs.

Sounds good for removing a column, but not if you want to see 10px of it.

3. td.style.visibility = 'collapse' the data cells: sort of works, but
not completely.

4. td.style.overflow = 'hidden' the data cells: seems to me this
should definitely work, but it doesn't.

Table layout is ... complicated. CSS even has two different layout
"algorithms" for tables to choose between:
<URL:http://www.w3.org/TR/CSS21/tables.html#width-layout>

You use the automatic table layout algorithm (the complex one). In
that case, setting the width of a table cell is really only setting
the min width. That layout algorithm will find a maximum width
depending on context, and find a layout of columns that somehow
fits all the constraints.

How can I set the width of the column to, say, .5em? (Naturally, I'm
really trying to set the width to that of a restore icon, but it seems
simpler to ignore that.)

Hmm, odd suggestion: Try wrapping the cell contents in an extra span.
This will basically be ignored in normal layout. When you want to
reduce the width of the cell, change the span to have:
width:10px; display:block; overflow:hidden; white-space: nowrap;
(preferably by just setting its className).

I did a quick-and-dirty test, and it works in at least Opera 9,
Firefox 3, Chrome 4 and IE 8 (but I'm assuming it won't work in IE 6).
(see <URL:http://www.infimum.dk/privat/columnshrink.html>)

/L

 

[Thomas 'PointedEars' Lahn]

Problem page: www.sellmycalls.com/option-chain-ex.html
Problem code: lines 344-389

Each column TH has a minimize icon on the left. It's meant to minimize
the entire column by shrinking its width to some small number, like
10px or .5em.

After I click the icon, however, the column width/offsetWidth/
clientWidth doesn't change; it remains stuck at 46 on my machine.

I have no inclination to debug your original code (which would involve a
lot more than just the lines you mentioned); create (and, if still
necessary, post) a reduced test case, and clearly state your target runtime
environments.

A few initial observations and suggestions (far from being complete),
though:

- I find your source code hard to read; consider

* using multi-line pre-comments instead of single-line post-comments;
use single-line comments only for deactivating comments, not
documenting ones, to distinguish them;
* aligning braces of /Block/ statements (that helps especially if you
would insist on the comment style);
* making it an exception for a line to exceed 80 characters, including
comments;
* removing spaces after opening and before closing parentheses
* always adding a space after control statements like `if', to
distinguish those statements from method calls;
* declaring variables where you use them first instead.

- Line 344 is (comments removed)

for ( i = 0; i < cols.length; ++i ) {

More efficient would be

for (i = 0, len = cols.length; i < len; ++i) {

(provided `len' was declared and `cols.length' did not change in the
loop). If order would not matter, you should consider a countdown
loop which is often more efficient:

for (i = cols.length; i-- {

This applies to other lines as well. (Whether `++i' is more efficient
than `i++' is questionable; I find the latter easier to read than the
former.)

- The identifier `getElementsByClass' might already be in use; consider
prefixing the identifier or "namespacing" the method.

- The first argument of columnHideShow() is named `theClass' but a
reference to the element having the value of the `class' attribute
is expected instead; consider renaming it.

- Consider placing each assignment it its own line instead, including
variable initializations.

- Repeated `findParentTH(theClass)' calls are inefficient if the value
of `theClass' does not change; consider assigning to `theClass' when
necessary and using the assigned value throughout the method.

- It is unnecessary, and less efficient, to create empty Arrays first
and later overwrite the stored reference, if the return value of the
method that is assigned to the property variable later already returns
a reference to an empty Array if there are no matches. This applies,
for example, to

var cols = [];
// ...
cols = getElementsByClass( th.className, main, '*' );

in lines 290 and 298, respectively. Consider writing either

var cols;
//
cols = getElementsByClass(th.className, main, '*');

or

var cols = getElementsByClass(th.className, main, '*');

instead.

- `main' is assigned the same value on each call of columnHideShow();
consider caching it to avoid calling the method right-hand side.

- The `tagName' property might be easier to recognize than the `nodeName'
property; it is sufficient here, and also slightly more compatible.

- In line 304, there is (comments snipped)

if( typeof hideState == 'undefined' ) {
var hideState = columns[j].hideState + 1;
}
columns[j].hideState = hideState & 1;

in which you augment a host object (referred to by `columns[j]') with
a property and read from it. This is unreliable, do not do it; use
wrapper objects instead, or store the state elsewhere but in a host
object.

All in all, I think a rewrite is in order.


PointedEars

 

[Garrett Smith]

You shouldn't keep your data in the table. For a web application, HTML
is presentation. Keep it in a data structure and rebuild the table
when necessary. That way the data can't get out of sync.

That advice is not generally applicable.

The 'DOM' is the model. If the document is semantic and well-structured,
then keeping the data in the page makes sense.

Consider degrading to non-js browsers (like googlebot).

Sounds good for removing a column, but not if you want to see 10px of it.



Table layout is ... complicated.

A type of CSS overflow for TR and TH and TD woud avoid such complexities.

CSS even has two different layout

"algorithms" for tables to choose between:
<URL:http://www.w3.org/TR/CSS21/tables.html#width-layout>

You use the automatic table layout algorithm (the complex one). In
that case, setting the width of a table cell is really only setting
the min width. That layout algorithm will find a maximum width
depending on context, and find a layout of columns that somehow
fits all the constraints.


Hmm, odd suggestion: Try wrapping the cell contents in an extra span.

An inline-level element such as SPAN cannot have block-level children.
Wouldn't DIV be a better choice here?

My usual suggestion for this situation is to not use a loop to apply
styles to descendants. Instead, add a class token to the nearest common
ancestor (common to the TH and the TD). In the source code, you will
need to add a style rule to the stylesheet with the selector text of
#ancestor-id .descendant-class and add the class attribute containing
that class token (or other attribute selector).

The above can be realized in two LOC of js with the help of a
toggleClass function:

function clickHandler(controlCell) {
var theTable = document.getElementById("theTable");
toggleClass(theTable, "hide" + controlCell.id);
}

table.hidec0 #c0 div,
table.hidec0 td[axis~=c0] div {
width: .5em;
overflow: hidden;
white-space: nowrap;
}

Why not use event delegation on the TR here?

 

[lcplben]

Hmm, odd suggestion: Try wrapping the cell contents in an extra span.
This will basically be ignored in normal layout. When you want to
reduce the width of the cell, change the span to have:
 width:10px; display:block; overflow:hidden; white-space: nowrap;
(preferably by just setting its className).

I did a quick-and-dirty test, and it works in at least Opera 9,
Firefox 3, Chrome 4 and IE 8 (but I'm assuming it won't work in IE 6).
(see <URL:http://www.infimum.dk/privat/columnshrink.html>)

Brilliant! Lasse, this works absolutely great! Thanks so much for
saving my hair. I'm learning, but slowly.

-- ben

 

[lcplben]

A few initial observations and suggestions (far from being complete),
though:

- I find your source code hard to read; consider

  * using multi-line pre-comments instead of single-line post-comments;
    use single-line comments only for deactivating comments, not
    documenting ones, to distinguish them;

good idea.

  * aligning braces of /Block/ statements (that helps especially if you
    would insist on the comment style);

style is migrated from C style so I'll keep it.

  * making it an exception for a line to exceed 80 characters, including
    comments;

yes, indeed.

  * removing spaces after opening and before closing parentheses
  * always adding a space after control statements like `if', to
    distinguish those statements from method calls;

also a style issue.

  * declaring variables where you use them first instead.

I had earlier thought that this practice was merely sloppy coding but,
on thinking about it, I see that it makes very good sense.

  More efficient would be

    for (i = 0, len = cols.length; i < len; ++i) {

  (provided `len' was declared and `cols.length' did not change in the
  loop).  If order would not matter, you should consider a countdown
  loop which is often more efficient:

    for (i = cols.length; i-- {

  This applies to other lines as well.  

yes, I've taken that suggestion.

(Whether `++i' is more efficient
  than `i++' is questionable; I find the latter easier to read than the
  former.)

just a (bad) habit of mine.

- The first argument of columnHideShow() is named `theClass' but a
  reference to the element having the value of the `class' attribute
  is expected instead; consider renaming it.

fixed, thanks.

- Consider placing each assignment it its own line instead, including
  variable initializations.

not sure what you mean, but it will surface in time.

- Repeated `findParentTH(theClass)' calls are inefficient if the value
  of `theClass' does not change; consider assigning to `theClass' when
  necessary and using the assigned value throughout the method.
fixed.

- It is unnecessary, and less efficient, to create empty Arrays first
  and later overwrite the stored reference, if the return value of the
  method that is assigned to the property variable later already returns
  a reference to an empty Array if there are no matches.  This applies,
  for example, to

    var cols   = [];
    // ...
    cols = getElementsByClass( th.className, main, '*' );

  in lines 290 and 298, respectively.  Consider writing either

    var cols;
    //
    cols = getElementsByClass(th.className, main, '*');

  or

    var cols = getElementsByClass(th.className, main, '*');

  instead.
fixed.

- `main' is assigned the same value on each call of columnHideShow();
  consider caching it to avoid calling the method right-hand side.
fixed.

- The `tagName' property might be easier to recognize than the `nodeName'
  property; it is sufficient here, and also slightly more compatible.

not too much I can do about this.

- In line 304, there is (comments snipped)

    if( typeof hideState == 'undefined' ) {
      var hideState = columns[j].hideState + 1;
    }
    columns[j].hideState = hideState & 1;

  in which you augment a host object (referred to by `columns[j]') with
  a property and read from it.  This is unreliable, do not do it; use
  wrapper objects instead, or store the state elsewhere but in a host
  object.

I see. OK.

All in all, I think a rewrite is in order.

I will be waiting for your submission

Thank you, PE, that was all very, very helpful.

-- ben

 

[lcplben]

Consider degrading to non-js browsers (like googlebot).

Yes, thanks. Googlebot sees all the indexable matter, which is static.
The dynamic stuff in this table is totally ephemeral, changing from
minute to minute, so and it bores Googlebot. As for end users, this
page will never in a jillion years run without js.

An inline-level element such as SPAN cannot have block-level children.
Wouldn't DIV be a better choice here?

div might be better, but I'm sorry I didn't mention that the parent of
the span is th/td so display:block works great.

My usual suggestion for this situation is to not use a loop to apply
styles to descendants. Instead, add a class token to the nearest common
ancestor (common to the TH and the TD).

That means thead and tbody; I'd love to do that and I'll try it.

In the source code, you will
need to add a style rule to the stylesheet with the selector text of
#ancestor-id .descendant-class and add the class attribute containing
that class token (or other attribute selector).

The above can be realized in two LOC of js with the help of a
toggleClass function:

function clickHandler(controlCell) {
     var theTable = document.getElementById("theTable");
     toggleClass(theTable, "hide" + controlCell.id);

}

table.hidec0 #c0 div,
table.hidec0 td[axis~=c0] div {
   width: .5em;
   overflow: hidden;
   white-space: nowrap;
}

Why not use event delegation on the TR here?

LOC? Delegation? I don't know enough to evaluate these last couple of
suggestions, but this gives me a chance to learn some more whiz-bang.
See, I almost care more for how the code looks and smells than how it
works

-- ben