emberjs / list-view

An incremental rendering list view for Ember.js
MIT License
465 stars 116 forks source link

Ember 1.13+ (Glimmer) Support #245

Closed ghost closed 6 years ago

ghost commented 9 years ago

Not sure if this has been raised but list view is broken with the latest Glimmer changes.

stefanpenner commented 9 years ago

Ya. We are aware. Just lack the time currently to update it. Hopefully soon :)

berislavbabic commented 9 years ago

@stefanpenner is there any low hanging fruit that I could help with here?

stefanpenner commented 9 years ago

@stefanpenner is there any low hanging fruit that I could help with here?

I don't believe so, the upgrade will require some effort. I would be excited if someone had time for it soon .

taras commented 9 years ago

I will have time to do it but the path forward has been muddled by the state of the pack branch and refactoring that Kris wants to do to make it Ember 2.0 compatible. Let's chat about it tonight.

stefanpenner commented 9 years ago

@taras sounds great :) See you at dinner!

taras commented 9 years ago

Looking forward to it :)

raytiley commented 9 years ago

@stefanpenner @taras Any news from dinner?

stefanpenner commented 9 years ago

@stefanpenner @taras Any news from dinner?

I plan to take an afternoon this week to dig into the current status of things, and we hope @krisselden is in-fact able to get some time to come up with an MVP we can role with.

raytiley commented 9 years ago

plan to take an afternoon this week to dig into the current status of things, and we hope @krisselden is in-fact able to get some time to come up with an MVP we can role with.

Sweet if there are any easy to explain items on the punch list feel free to post them and I can try to tackle as time allows

stefanpenner commented 9 years ago

Sweet if there are any easy to explain items on the punch list feel free to post them and I can try to tackle as time allows

ya, i haven't even gotten to that phase yet

tschoartschi commented 9 years ago

I'm very interested in using "list-view", so are there any news on this issue?

mrinterweb commented 9 years ago

Is list-view still incompatible with 1.13?

stefanpenner commented 9 years ago

i believe @krisselden and @raytiley are working on it

ivan-granulin commented 9 years ago

@stefanpenner @krisselden @raytiley Will the new list-view be backwards compatible with the current one (version 0.5)?

Will there be any syntactical changes? In other words - will I have to change the code of my app that's using list-view 0.5?

raytiley commented 9 years ago

@mrinterweb - @krisselden and I spiked out a quick and dirty compatible version, definitely not production ready. The internals are much simpler now, but there is still work needed to get it releasable. I'm on vacation all of next week, but I hope to really dig into after that and get something out that is workable soon.

@ivan-granulin you'll almost definitely need to update your app. In the current spike here is what the syntax looks like:

{{#ember-list
            items=model
            height=500
            width=800
            cell-layout=(fixed-grid-layout 800 25)
            as |run|}}
  {{! Your markup goes here }}
{{/ember-list}}

So basically the ember-list works the same as each but takes some additional properties. The interesting one is the cell-layout property which takes the result of a helper to determine the layout. This means you can inject different layouts pretty easily. Think rows vs grids.

ivan-granulin commented 9 years ago

@raytiley Thank you for reply. I do really appreciate your work. The syntax looks just superb! What a pity we have to wait till July 24 to get it in our hands - having said that, do you plan any "pre-July-24" release of ember-list?

By the way, if the ember-list works basically the same as each, does that mean it could replace it when rendering an html's table-based grid (maybe with some modifications)? - Example:

<table>
 {{#ember-list items=model height=500 width=800 cell-layout=(fixed-grid-layout 800 800) as |item|}}
    <tr>
       <td>{{item.prop1}}</td>  <td>{{item.prop2}}</td>  <td>{{item.propN}}</td>
    </tr>
 {{/ember-list}}
</table>

Edit: Is it possible for you to share the "quick and dirty compatible version" of Glimmer-ready ember-list?

sberan commented 9 years ago

:+1: to a pre-release if available I'd love to beta-test.

mrinterweb commented 9 years ago

I'm also available to help test.

troyhalden commented 9 years ago

@raytiley What's the current status of ember-list? Could you please share a .zip version?

mrinterweb commented 9 years ago

It would be easier to test from a feature branch.

troyhalden commented 9 years ago

Had I had noticed before... The new, Glimmer-ready ember-list is available at https://github.com/krisselden/list-view/tree/2.0.0

karanjthakkar commented 9 years ago

@ivan-granulin Is this going to be released today? Eagerly waiting for it.

ivan-granulin commented 9 years ago

@karanjthakkar Unfortunately, I'm not a member of the core team so I don't know when it will be released.

But looking at https://github.com/emberjs/list-view/network you'll see that version 2.0.0 can be found at https://github.com/krisselden/list-view/tree/2.0.0 - as pointed out by @troyhalden.

shaunc commented 9 years ago

I checked out https://github.com/krisselden/list-view/tree/2.0.0 and tried to run tests -- unsuccessfully (see below). I'm not sure if this is due to unstability of canary build of ember of the new list-view, something in my local environment, or in a related package. (Any suggestions?)

Failures (23 -- 47) were mostly like 23 --

not ok 23 PhantomJS 1.9 - TestLoader Failures: dummy/tests/unit/content-test: could not be loaded

    actual: >
        null
    message: >
        Died on test #1     at http://localhost:7357/assets/test-support.js:2887
            at http://localhost:7357/assets/test-support.js:5652
            at http://localhost:7357/assets/test-loader.js:31
            at http://localhost:7357/assets/test-loader.js:21
            at http://localhost:7357/assets/test-loader.js:40
            at http://localhost:7357/assets/test-support.js:5659: Could not find module `qunit-module` imported from `dummy/tests/helpers/module-for-view`
    Log: >
...

Failure 27 was a bit different:


not ok 27 PhantomJS 1.9 - TestLoader Failures: dummy/tests/unit/initializers/list-view-helper-test: could not be loaded

    actual: >
        null
    message: >
        Died on test #1     at http://localhost:7357/assets/test-support.js:2887
            at http://localhost:7357/assets/test-support.js:5652
            at http://localhost:7357/assets/test-loader.js:31
            at http://localhost:7357/assets/test-loader.js:21
            at http://localhost:7357/assets/test-loader.js:40
            at http://localhost:7357/assets/test-support.js:5659: Could not find module `dummy/initializers/list-view-helper` imported from `dummy/tests/unit/initializers/list-view-helper-test`
    Log: >
...

NB during pull before tests:

Please note that, ember-list-view depends on ember#beta which resolved to ember#cfd23c6f18 ember-cli-shims#0.0.3, ember-load-initializers#0.1.4 depends on ember#>=1.4 <2 which resolved to ember#1.13.5 ember-resolver#0.1.18 depends on ember#> 1.5.0-beta.3 which resolved to ember#1.13.5 Resort to using ember#beta which resolved to ember#cfd23c6f18 Code incompatibilities may occur.

raytiley commented 9 years ago

@shaunc What is implemented so far is still proof of concept. I'll be circling around soon and writing / updating tests.

krisselden commented 9 years ago

@shaunc to reiterate what @raytiley said, there is only one template we were hacking on https://github.com/krisselden/list-view/blob/2.0.0/tests/dummy/app/templates/simple.hbs which is what you should look at if you are looking to try it out.

Also, will be moving to a branch on this repo soon. As for 1.13 support, I plan for it to pass the 1.13 tests, I'm just not sure I plan for IE 8 support.

shaunc commented 9 years ago

Sure - - Thanks. With so much flux in the ecosystem I thought it could be a package.json with wrong version of something.

raytiley commented 9 years ago

@krisselden @stefanpenner I'm going to start working on some cleanup and test migration for the glimmer support. Is the end goal to have a single version that provides 1.12 support and 1.13+ (glimmer) support? Or should all the legacy views be removed? I guess another option would be a brand new addon renamed ember-list (since views are evil right). Just looking for some direction.

krisselden commented 9 years ago

No views

mbrakken commented 9 years ago

Thanks for the work on the 2.0.0 branch @krisselden and @raytiley. I currently have a version in production with Ember 1.13.6, and it seems to be pretty happy. Certainly happier than not using ember-list. I hope to try it with Ember 2 soon, and I'm excited to see the eventual release.

I haven't dug into this yet, but I just wanted to leave a note that the 2.0.0 branch currently doesn't work with ember 1.13.7 and 1.13.8 (initial content renders but doesn't update, and declaring widths on cells doesn't result in a grid), and with ember-cli 1.13.8 the layout import in addon/components/ember-list.js, import layout from './ember-list-template'; fails.

I hope to find some time to look into those issues, but I'm not sure when.

shaunc commented 9 years ago

@mbrakken -- I've been working off krisselden/raytiley branch, trying to get the tests working. Are you using the same block interface as they are, and have you worked on the tests yet? I'd be eager to merge if it makes sense, and even better to throw out my work if its superfluous :). I'm most eager to get ember-table working with glimmer, and it's supposed to be based on ember-list now. If you have a public repo please post a link!

Thanks!

mbrakken commented 9 years ago

@shaunc I am using the block interface, but I haven't touched tests in any way. Partially I wasn't sure how much thrash there'd still be on the branch, so I just went with getting things functional. Also, I need to get better at bothering to actually write tests for ember :(

The repo / branch I'm working off is here: https://github.com/mbrakken/list-view/tree/2.0.0. I only made a few rather small modifications so far.

shaunc commented 9 years ago

@mbrakken -- great - thanks! It looks like we've made a few similar changes. :) I'm trying to slog my way through the tests ("learning by doing": seems that there are some undocumented changes in test infrastructure) though I must admit I'm slacking a bit on some. (Do we really need a test that the component properly bubbles actions, for instance? [controller-test.js] Presumably this had something to do with the details of the old implementation. For now in my branch I've dropped this test; we'll see if project owners will complain.)

I'm not sure if I'll get all the way through, but I'll post something at the end of the day and include a link here regardless.

mmun commented 9 years ago

@mbrakken @shaunc Hey folks! I've "officially" moved kris' branch to https://github.com/emberjs/ember-collection and started with some cosmetic changes. The first order of business is getting some basic tests passing. Please contribute there! :) If you want to chat you can hop into #dev-ember in the Ember community slack channel.

shaunc commented 9 years ago

Thats great! I've manually merged in my changes and posted here. https://github.com/shaunc/ember-collection/tree/tests-2.0. Various changes (which may or may not be a good idea :)). In particular I'm now observing the array for changes to get "content-test" working. Is this how it should be done?

Right now this is still very far from being a pull request. (e.g. jshint is a mess.) As there is activity now I thought I'd post what I have.

I have "content-test" running -- comments/suggestions welcome. I threw out "controller-test" (comments also welcome :)), and am currently puzzling over "list-view-test".

I'm doing my best to use standard testing tools only... avoiding moduleForView etc. Perhaps I should generate an acceptance test skeleton for "list-view-test"?

mmun commented 9 years ago

@shaunc Can we start with component integration tests instead of acceptance tests?

shaunc commented 9 years ago

@mmun The first test I did was "content-test" which is integration. I was thinking about list-ivew-test next just to get the general structure down. I'd be happy to go on to other integration tests... but am trying to get rid of "moduleForTest" everywhere so thought I'd ask about the skeleton.

(NB I'm trying to adapt old tests where reasonable rather than write new ones. Probably some new ones will be a good idea as well, but adapting old where possible will be a good diagnostic to users of the old component.)

mmun commented 9 years ago

Awesome :+1: I left some comments on your commit.

shaunc commented 9 years ago

Thanks!

I am looking into "multi-height-list-view" test now. It seems the old implementation used "heightForIndex" function. Now it looks like "layout-bin-packer" just reads the "height" property off the content.

Bin.prototype.heightAtIndex = function (index) {
  return this.content[index].height;
};

Are we comfortable with that? Or should there be an attribute that provides a function to get a height?

mmun commented 9 years ago

Not sure. Either way seems fine to me. We can revisit it.

mmun commented 9 years ago

@shaunc You should get the tests passing and submit a PR so we can iterate together. Feel free to replace any failing QUnit.tests with QUnit.skip (via import { skip } from ...) and comment out whatever you need to.

shaunc commented 9 years ago

@mmun -- ok -- slashed & burned; submitted. :) It seems to me that to deal with the concerns of "multi-height-list-view" the layout should be the thing that can figure out item heights. In particular, its silly to adapt these tests for "fixed-grid". What is the "mixed-grid" layout supposed to do? It looks like it should put items of various sizes on "shelves", wrapping when necessary. Perhaps I should adapt the "multi-height-list-view" tests to be tests of "mixed-grid"?

shaunc commented 9 years ago

NB: https://github.com/shaunc/ember-collection/tree/phantom-offset and https://github.com/stefanpenner/layout-bin-packer/issues/9

The branch has more tests including a failing one demonstrating bug in layout-bin-packer

mansona commented 9 years ago

So is https://github.com/emberjs/ember-collection now the "official" successor to ember-list-view? If so can we create a meta issue over there that people can follow for updates on it's 1.13 and 2.0 readiness?

I'm only asking because to me (as an outside observer) it is almost impossible to track the progress of this issue.

shaunc commented 9 years ago

@masona - As long as we assume that @mmun isn't a fiendish impostor (and if he is he's playing a long game, because he seems like a pretty nice guy), then I think you're right. I'm eager to get this finished because I want to use a glimmer-based ember-table in my project, so am pushing ahead. If you open an issue over there I'll be glad to post status to it (as far as it depends on me -- I'm not an official contributor).

rwjblue commented 9 years ago

Yes, ember-collection will be replacing list-view.

I definitely agree that we should make a meta issue in ember-collection repo. I am unsure of the major points so I'll leave that to @mmun.

tim-evans commented 9 years ago

Any reason we just didn't rename this repository to ember-collection?

rwjblue commented 9 years ago

@tim-evans - I suggested against it (@mmun would have preferred that too). Sorry about the confusion. :disappointed:

My thought process was that the fundamental concept of the new "thing" is roughly the same was previously published versions of list-view, but it really is a whole new beast.

Again, sorry if my suggestion was ultimately a mistake...

mansona commented 9 years ago

@rwjblue I think ultimately your suggestion to make a new repo was the right choice, and again in the same sentiment as my last question it just needs to be documented somewhere :joy: A simple update of ember-list-view's Readme pointing to ember-collection's Readme should solve the issue for any brave adventurers straying here in the future.

I think we're agreed that we should create an issue on ember-collection. Although I do agree with @rwjblue that @mmun should be one to create the issue seeing as he is closest to the implementation I just went ahead and did it because:

  1. I believe in "ask forgiveness not permission" and
  2. As @mmun hasn't responded yet I figured he was busy and I wanted to take some of the pressure off.

If I become a bottleneck here we can just create a new one over then :wink:

Here is the link: https://github.com/emberjs/ember-collection/issues/5

rwjblue commented 6 years ago

I'm sorry we didn't get back to this previously, but at this point this repo is essentially unmaintained. Please use @html-next/vertical-collection or ember-collection for similar functionality.

Closing...