Height tests are broken

[BryanCrotaz]

I've fixed some tests, but those where the grid is supposed to fit the parent's available space are broken.

I'm not sure I like the algorithm you've used in calculateHeight which works out how the grid fits in its parent. The method you've used wouldn't update if the parent size changed.

Is there any reason why we couldn't set height: 100% instead?

[shaunc]

Hmm... I guess that if we are in "take up space in parent mode" we could use height=100%... however, body still needs explicit height I think, so need to listen to resize of parent....

[shaunc]

glanced at this ... didn't figure out how they broke yet though -- some changes I don't understand without a bit more time to think.

[BryanCrotaz]

They have always been broken

Bryan Crotaz
Silver Curve

On 14 Sep 2015, at 07:26, Shaun Cutts notifications@github.com wrote:

glanced at this ... didn't figure out how they broke yet though -- some
changes I don't understand without a bit more time to think.

—
Reply to this email directly or view it on GitHub
#25 (comment).

[shaunc]

well --- were working for me when I pushed them, but aren't working now. Of course could have been always broken for you on windows/IE

[shaunc]

(for that matter, tests used to work in firefox... :) ... hmm... I haven't figured that out yet either)

[BryanCrotaz]

I made some changes to the way the asserts were done last week which fixed
a number of them

Bryan Crotaz
Silver Curve

On 14 Sep 2015, at 07:40, Shaun Cutts notifications@github.com wrote:

(for that matter, tests used to work in firefox... :) ... hmm... I haven't
figured that out yet either)

—
Reply to this email directly or view it on GitHub
#25 (comment).

[shaunc]

It used to be that element heights (header/body/footer) were set explicitly. They seem not to be anymore, but the code depends on this, I think.

[shaunc]

For instance:

_bodyShellHeight() {
  // calclulate the height of content except contents of body
  var headerStyle = this._actualHeaderStyle();
  var bodyStyle = this._actualBodyStyle();
  var footerStyle = this._actualFooterStyle();
  var _N = val=> val == null ? 0 : parseFloat(val);
  var shellHeight = 0;
  shellHeight += _N(headerStyle.offsetHeight) - _N(headerStyle.bottomBorderWidth);
  shellHeight += Math.max(
    _N(headerStyle.bottomBorderWidth), _N(bodyStyle.topBorderWidth));
  shellHeight += Math.max(
    _N(footerStyle.topBorderWidth), _N(bodyStyle.borderBottomWidth));
  shellHeight += _N(footerStyle.offsetHeight) - _N(footerStyle.topBorderWidth);
  return shellHeight;    
},

The idea was that user might want to use css on header/body/footer -- and we should respect if so -- so I read computed heights. However, now the header is 0 height and content within it is visible because of overflow. I'm not sure how we want to resolve.

[shaunc]

Do you know why the header & footer are set to 0 height? I could go down to look at cell height. What I haven't figured out is how the body gets its height currently.

[shaunc]

In chrome seems to figure out height of header despite css. But gets confused in different ways in both safari and firefox... I'll try to look more carefully at it after I get some sleep. It would be helpful for me to understand how you think it should work. I thought the idea was to read the height of "rest" (excluding body) and calculate body height from that.

But how is height of "rest" being determined? It would seem that in chrome (and mostly in safari, though there seems to be some other glitch) it suffices that the header cells have content of given height.

In firefox for me, the "filler" element in header gets rendered to height 400px and this pushes out calculated height of header itself. Hmm...

Going to bed, now -- ... if you could write a note on how the heights of header/footer are supposed to be set that might help me tomorrow.

[BryanCrotaz]

table layout is used. If height is zero the row auto sizes to content.
Remaining rows split remaining space evenly.

Bryan Crotaz
Silver Curve

On 14 Sep 2015, at 08:02, Shaun Cutts notifications@github.com wrote:

Do you know why the header & footer are set to 0 height? I could go down to
look at cell height. What I haven't figured out is how the body gets its
height currently.

—
Reply to this email directly or view it on GitHub
#25 (comment).

[shaunc]

(came back up after brushing teeth :) ) hmm.. so what is now happening in firefox is that header (for scroll example) or header & footer (for overview table) are expanding to split rest of space (ie all of space) equally regardless of their content.

[shaunc]

hmm.... http://www.tads.org/t3doc/doc/htmltads/tables.htm ... seems that height is proportioned according to min-height (which will be content height).

But ... is this a good idea? I guess the first question should be: what interface to the user do we want to have to specify header/body/footer heights?

I thought that css probably best -- I envisioned it as:

#my-section .ember-grid .header { height: 20px; }

---

Problem is that firefox seems to be doing things correctly as far as I can tell - that is: it distributes the height according to the proportions of the heights of the elements. But then user spec of height actually becomes min-height, and no space for body is left over.

Is there some other reason to use table layout? It would seem to be easier to use block layout and have default height auto. But I may be missing something.

[BryanCrotaz]

It was the result of a full day of trying and failing to get it to lay out.
Will try with blocks and height auto

Bryan Crotaz
Silver Curve

On 14 Sep 2015, at 09:15, Shaun Cutts notifications@github.com wrote:

hmm.... http://www.tads.org/t3doc/doc/htmltads/tables.htm ... seems that
height is proportioned according to min-height (which will be content
height).

But ... is this a good idea? I guess the first question should be: what
interface to the user do we want to have to specify header/body/footer
heights?

I thought that css probably best -- I envisioned it as:

#my-section .ember-grid .header { height: 20px; }

---

Problem is that firefox seems to be doing things correctly as far as I can
tell - that is: it distributes the height according to the proportions of
the heights of the elements. But then user spec of height actually becomes
min-height, and no space for body is left over.

Is there some other reason to use table layout? It would seem to be easier
to use block layout and have default height auto. But I may be missing
something.

—
Reply to this email directly or view it on GitHub
#25 (comment).

[shaunc]

(Other possibility perhaps is to draw with overall height auto & table layout with no body, then measure & calculate body height, then set body height and also overall height ... if doesn't work with blocks & height auto we could fall back on this I suppose)

[shaunc]

anyway -- now really to bed.

[BryanCrotaz]

Measurement techniques don't deal with outer size changing in future. Eg in
a draggable split pane.

Bryan Crotaz
Silver Curve

On 14 Sep 2015, at 09:19, Shaun Cutts notifications@github.com wrote:

(Other possibility perhaps is to draw with overall height auto & table
layout with no body, then measure & calculate body height, then set body
height and also overall height ... if doesn't work with blocks & height
auto we could fall back on this I suppose)

—
Reply to this email directly or view it on GitHub
#25 (comment).

[shaunc]

Measurement techniques don't deal with outer size changing in future. Eg in
a draggable split pane.

That is true -- but in any case we need to do some measurement to setup body, as we can't rely on its content given we want overflow to scroll. So to deal with changing size in future we need listeners.

[BryanCrotaz]

So to deal with changing size in future we need listeners.

Not if we use CSS to do it. That was the point of going down the display:table route (which didn't work). I'm now investigating the auto size block method you suggested.

[shaunc]

An idea:

Position all three elements "absolute" with header and footer using top:0 and bottom:0 respectively. Body
would be in container with transparent background and padding so that header and footer could be seen. Body would then be rendered at height:100% in that container.

Would that work?

[shaunc]

Would still require measuring header and footer height to set padding, but assuming their sizes don't change wouldn't need us to measure the whole. (If their sizes do change we are in trouble anyway AFAIK). The container around "body" would be rendered w/ height 100% -- which should work according to http://stackoverflow.com/questions/14217783/set-height-100-on-absolute-div (if mysterious float:left is added to display?)

[BryanCrotaz]

I'll be playing tonight

[shaunc]

https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Flexible_Box_Layout/Using_CSS_flexible_boxes

[BryanCrotaz]

Rewritten using flexbox. Works beautifully on Windows Chrome. Please test on mac.

[shaunc]

much better but still some glitches.
0) both chrome and firefox support

1. Currently safari is not working at all but apparently supports flex with --webkit- prefix. Will test.
2. Problem with "native scroll" mode:

(Image partly scrolled NB there are scrollbars that appear on drag and when first drawn.) I saw this before with table layout and was able to fix....

---

Overall looks great -- good job!

We will probably want a polyfill to push back flex support for older IE (would be good to get back to IE9 to line up with ember).

[shaunc]

checked in fixes for safari support & native scroll

Safari has an unrelated problem which can nevertheless be solved by another application of flex to .row. Before:

After:

Does this work for you over in windows?

[BryanCrotaz]

broken in all sorts of ways on Windows -

• extra empty footer box
• column widths are different in body and header
• no y-scrollbar

[shaunc]

hmm... the intention was to condition differences in layout with ".native-scroll" which should only turn on if a scrollbar has 0 width in test (signalling that it renders "on demand" on top of content rather than beside it). However I used layout of columns w/ flexbox unconditionally, as it seemed that your tests indicated flexbox worked, and this fixed safari without breaking chrome or firefox on mac.

Is this mode turning on (do you see a native-scroll class on ember-grid element, or did I screw up the css in some way not guarded by .native-scroll... or both?) If the latter I will go back to the previous version you checked in and more rigorously ensure that changes are guarded by ".native-scroll". If the former, then some tweak to the code that detects default scrollbar width is needed.

[shaunc]

Just pushed something that might fix

• column widths are different in body and header

Was using flexbox layout for body rows as that fixed safari without changing anything on chrome/firefox. But it probably could change something.

Remaining:

• extra empty footer box
• no y-scrollbar

Might be linked -- is it possible the extra empty footer box is where the y-scrollbar should be and its not displaying? This sounds like either native-scroll mode is turning on when it shouldn't or some other change to css is not properly conditioned. However a diff seems to show other changes protected by &.native-scroll. Can you tell me if its turned on in your broken examples?

[shaunc]

Just pushed a candidate fix for

• extra empty footer box

I was piggybacking on check of scrollbar width in scroller-controller to set native-scroll mode -- but this required turning on control when too many rows to display as well as too many columns. Now have pushed fix to turn display off if not too many columns.

Probably deserves refactoring at some point to do scrollbar width check separately from scroll control.

I'm still not clear why you get

• no y-scrollbar

in windows (if in fact you still do get it).