-
Notifications
You must be signed in to change notification settings - Fork 4
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
Height tests are broken #25
Comments
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.... |
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. |
They have always been broken Bryan Crotaz On 14 Sep 2015, at 07:26, Shaun Cutts [email protected] wrote: glanced at this ... didn't figure out how they broke yet though -- some — |
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 |
(for that matter, tests used to work in firefox... :) ... hmm... I haven't figured that out yet either) |
I made some changes to the way the asserts were done last week which fixed Bryan Crotaz On 14 Sep 2015, at 07:40, Shaun Cutts [email protected] wrote: (for that matter, tests used to work in firefox... :) ... hmm... I haven't — |
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. |
For instance:
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. |
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. |
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. |
table layout is used. If height is zero the row auto sizes to content. Bryan Crotaz On 14 Sep 2015, at 08:02, Shaun Cutts [email protected] wrote: Do you know why the header & footer are set to 0 height? I could go down to — |
(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. |
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:
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. |
It was the result of a full day of trying and failing to get it to lay out. Bryan Crotaz On 14 Sep 2015, at 09:15, Shaun Cutts [email protected] wrote: hmm.... http://www.tads.org/t3doc/doc/htmltads/tables.htm ... seems that But ... is this a good idea? I guess the first question should be: what 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 Is there some other reason to use table layout? It would seem to be easier — |
(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) |
anyway -- now really to bed. |
Measurement techniques don't deal with outer size changing in future. Eg in Bryan Crotaz On 14 Sep 2015, at 09:19, Shaun Cutts [email protected] wrote: (Other possibility perhaps is to draw with overall height auto & table — |
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. |
Not if we use CSS to do it. That was the point of going down the |
An idea: Position all three elements "absolute" with header and footer using top:0 and bottom:0 respectively. Body Would that work? |
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?) |
I'll be playing tonight |
Rewritten using flexbox. Works beautifully on Windows Chrome. Please test on mac. |
much better but still some glitches.
(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). |
broken in all sorts of ways on Windows -
|
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 |
Just pushed something that might fix
Was using flexbox layout for body rows as that fixed safari without changing anything on chrome/firefox. But it probably could change something. Remaining:
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 |
Just pushed a candidate fix for
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
in windows (if in fact you still do get it). |
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?The text was updated successfully, but these errors were encountered: