shaunc / ember-grid

declarative table for ember
MIT License
15 stars 4 forks source link

Nub of rendering challenge #2

Closed shaunc closed 9 years ago

shaunc commented 9 years ago

Given

  {{#eg-body}}
    {{my-component value="name"}}
  {{/eg-body}}

We need to render

    {{my-component value="name"}}

In the context of the individual row. So:

1) look up contents of "eg-body" for given column 2) render contents in a custom wrapper component

What is relevant "contents of 'eg-body'? The dom nodes aren't sufficient. What we really want is the template (template string)? The question is, is that available?

BryanCrotaz commented 9 years ago

Yes that's the nub I was heading towards but you found the detail. Hmm...

BryanCrotaz commented 9 years ago

Wonder if we can yield it in somehow. Set the render context to off screen then yield

shaunc commented 9 years ago

In theory, layout of eg-body looks like

{{yield}}

anyway. The question will be, does instance of eg-body have a "template" attribute which we can simply assign to a dynamically created component whose purpose is to pass in the context to template.

BryanCrotaz commented 9 years ago

Isn't this exactly what #each has to do?

BryanCrotaz commented 9 years ago

Yes! It has layout property. See eg-body component def

shaunc commented 9 years ago

each has a static template... we have to do the same thing but dynamically. So can we do:

{{#ember-list as |column|}}
   {{display-cell tempate=column.cellTemplate}}
{{/ember-list}}
shaunc commented 9 years ago

layout is for the wrapper, template is for the contents of the wrapper.

shaunc commented 9 years ago

So task 1 is to write a little test w/

{{#x-foo}} something {{/x-foo}}

{{#ember-list as |foo|}} {{render-it template=foo.template}} {{#ember-list}}

... [will try it out -- now I'm being called away from the keyboard :)]

shaunc commented 9 years ago

hmm... so in a block component, template is null, and layout contains an already compiled htmlbars structure, which itself just contains the "yield". Of course, the DOM is present, but to get back to the actual template we would need to reverse compile.

What we really want (or at least what I was thinking about) is an htmlbars block helper that would grab the unrendered template callback, and save it for us to call back once we have set up the context.

However, the interfaces to this were called private and unstable. I think that (e.g.) in eg-body we actually have to iterate over the data and pass it in. If necessary we can use ember-wormhole to rearrange the content. I'll tweak the readme slightly and try to get an example of this working.

BryanCrotaz commented 9 years ago

Yes we definitely need it before rendering. How does #each do it? Isn't that doing the same thing, repeatedly rendering the same template with different injected data?

BryanCrotaz commented 9 years ago

Gotcha.

Can't use block helpers any more so we have to use wormhole technique. Can't use wormhole itself as it requires an elementId. So I've used the same technique in eg-render/eg-header-cell and eg-render/eg-footer-cell. Works beautifully.

Default header is column.header. Default footer is column.footer.

Now we just need to get the body working. I suggest you do that as you know ember-collection better than me. I'll get header and footer rows looking nice and getting column resize to work.

shaunc commented 9 years ago

each -- I looked at the code and is pretty simple. It wouldn't be hard to copy. The problem is whenever they change the private APIs and #each breaks, they have to fix it. But they don't have to fix ember-grid.

I think we can use wormhole: we can create an id from row*column. And I think we have to to keep the current form, as if we keep the bodies inside the columns, they won't render in the right order.

This is my plan:

1) Render data fields in columns (with display = none)

2) Render a sheet of divs with ids

Both of the above with ember-collection.

3) Teleport the data fields into place with wormhole. I naively believe that if a row*column doesn't exist in the destination grid, the wormhole will simply fail without bad side effects.

shaunc commented 9 years ago

If you have a better idea tell me.. possibly we could leave as columns and render the whole as a column table, but it won't be what users would expect at least.

BryanCrotaz commented 9 years ago

each can't work. We can't do our own block helpers any more. Only ember core can do that.

wormhole can't work as we need unique ids - what happens if the user wants two tables?

I've used the same technique as wormhole, and made it work nicely for headers and footers (including fallback to default string)

BryanCrotaz commented 9 years ago

take a look at the currently checked in code and see what you think.

shaunc commented 9 years ago

A random string would do as a prefix ... but checking out the code now.

shaunc commented 9 years ago

Ok .... yes that is good for headers and footers. We can move things around "relatively" rather than absolutely using parent. ... but that isn't the hard part with the body (unique ids per se aren't a problem): here body cells can render a column of fields, but we can't leave them like that -- for instance we need a scrollbar for whole sheet rather than one for each column, so we have to undertake more transformative surgery.

I didn't get as much done as I expected as we have guests, but will consider both techniques. Right now I'm just getting to the point where I have two structures: one with fields in columns, and another with an empty grid of rows & columns. I'll rebase based on your work before I continue -- no need for teleportation when normal transportation already works. Another part I haven't quite worked out yet is sequencing: I can't move elements into the grid before it exists.

shaunc commented 9 years ago

Its gotten late and I didn't get done as much as I hoped as a few other things came up. I haven't merged body work yet. Just pushed version with your components in pod structure so we don't diverge too much. Tomorrow have "real work" until evening; hopefully can push something then. Thanks!

BryanCrotaz commented 9 years ago

I think we should find a way to use ember container for the body. It's huge and we shouldn't rewrite it.

shaunc commented 9 years ago

ember-collection, you mean?

shaunc commented 9 years ago

Ok -- have pushed a version -- shows body rendering. It is still quite buggy: ember-collection and own code are both fiddling with contents dynamically and the interference makes a mess on scroll. But illustrates the basic technique.

BryanCrotaz commented 9 years ago

I've cleaned up jshint and simplified some of the structure. Wondering if we can eliminate ember.run.next to speed up rendering

BryanCrotaz commented 9 years ago

I'll see if I can get drag-resize headers next

shaunc commented 9 years ago

Great -- I'll check it out. I've found a few bugs in ember-collection which might or might not help on scroll. (Still fixing now....) One more complex thing thing we need to get working is horizontal scroll if the columns push beyond the viewport. What is your npm username so I can add you as npm owner?

BryanCrotaz commented 9 years ago

bcrotaz

shaunc commented 9 years ago

(added in npm)

BryanCrotaz commented 9 years ago

horiz scroll needs to be on all three zones, so ember-collection and hread and footer row should have min-width = sum(column widths) + borders and there should be a scrollbox outside

shaunc commented 9 years ago

yes... I was also thinking of using ember-collection in definition with horizontal window so that we don't have to draw all the columns but only those that fit.

As for borders, I'm wondering if we can get away with leaving that to the user - just render the cells absolutely with no gaps. and let them style eg-cell-body (for example) and and/or whatever else the have actually rendered.

BryanCrotaz commented 9 years ago

no, css is king here. we can query css params to see how much space to leave. Take a look at ember-split-view in the minSize computed property. Then the user can just use css to style and we use whatever they used.

shaunc commented 9 years ago

yes -- very good. Btw perhaps its time to start adding tests? Seems like we have "proof of concept".

BryanCrotaz commented 9 years ago

I think we need to prototype a couple more things first. It's very slow to start up - all the run.next cycles - can we get involved in the rendering so it renders correctly first time?

shaunc commented 9 years ago

One thing I'm going to try now is to eliminate the per-column ember-collections body definitions -- no need to use frame animation inside of display==None. Instead we should just render just a slice. ...but its already getting to have non-trivial complexity so lets at least try not to add too many more features before working on the tests.

BryanCrotaz commented 9 years ago

That's weird - the divs in the collection are in reverse order/

Not at all sure what your latest comment means

BryanCrotaz commented 9 years ago

made some nice progress. We need to think about how body cells are laid out - I think your method is about to hit a brick wall when we try and have the column know about styling to calc offsets. What's wrong with inline-block?

shaunc commented 9 years ago

merging changes -- about to check in (so you can see what it means) (EDIT oh -- something else has come up -- back in an hour).

I thought about inline-block ... but if we really want to use ember-collection, it places everything absolutely and would be quite difficult to get around.

wrt to:

classNames: ['eg-body-cell'],

Can we make sure to namespace our css (it doesn't have to be eg-body-cell -- could be eg-cell for instance if you want.

shaunc commented 9 years ago

wrt to the divs in the collection are in reverse order/ -- there is a scrolling bug now. Problem is that ember-collection keeps a hash of elements -- not necessarily in order and then places them and tells them to render as needed. However, render for me means "take your content from the definition" and now if another cell has already taken it I'm stuck with the old content. I need to create another map.

shaunc commented 9 years ago

wrt classNames: ['cell']

nevermind... I guess thats ok.

BryanCrotaz commented 9 years ago

My thinking is that we're namespaced within ember-grid and by using less we enforce that

BryanCrotaz commented 9 years ago

Also wondering whether the grid should have two drawing modes. One for a normal table where columns are all visible and one spreadsheet style where a subset of cols are visible. Former doesn't need an ember-collection per row

shaunc commented 9 years ago

Yes -- I decided that it was ok. The only thing is if user drops in a table inside something styled #my-div .cell { ... } but is easy to fix and OTOH convenient to use. Just I currently use selector to look up cell, and will add eg-body-cell as well.

WRT to css styling of cells. how do you want to do it? It seems to me that column.width has to govern here...?

BryanCrotaz commented 9 years ago

You can look up cell with '.ember-grid .body .cell' or you can start find within body or row for speed

BryanCrotaz commented 9 years ago

I've already set cell width from column width

shaunc commented 9 years ago

Former doesn't need an ember-collection per row

The idea would 2 ember collections: one large one which ranges over whole including definitions and governs horizontal styling, and one internal one over rows which governs vertical scrolling. However, we could leave out the large one possibly -- lets get the vertical scrolling working and then consider.

I've already set cell width from column width

what do you see as the problem w/ absolute positioning?

BryanCrotaz commented 9 years ago

I've bodged it quickly with a style binding but need to do it properly later to stay within CSP rules

shaunc commented 9 years ago

ok -- but that isn't a problem with absolute position per se, right -- we need to do it right however? (btw you might have noticed I tossed in "unsafe-self" in dev to get rid of annoying reports)

BryanCrotaz commented 9 years ago

The problem with absolute positioning is to bring in CSS padding margins and borders. CSS might be overridden in app. The way to do that is in the cell component. Maybe col width is just the content width of the column. Then the cell component adds on CSS widths

shaunc commented 9 years ago

I thought of "eg-body-cell" as a wrapper around the "real cell" which is whatever the user wants it to be (and would be best target of css).

shaunc commented 9 years ago

we could explicitly render a wrapper and also a cell perhaps, and use the cell for positioning?

BryanCrotaz commented 9 years ago

Given the number of cells we ought to minimise dom complexity. And just do what is necessary.

BryanCrotaz commented 9 years ago

I wonder if we should start with body rows using the same technique as header rows