Skip to content
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

Update ember-in-viewport #674

Conversation

kevinansfield
Copy link

@kevinansfield kevinansfield commented Mar 15, 2019

no issue

  • bumps ember-in-viewport too 3.2.2
  • adds a fix for passing non-existent selectors to ember-in-viewport as occurs when there is no fixed header/footer and therefore no .lt-scrollable element

This was a necessary upgrade for us because the pinned version of ember-in-viewport has a bug in Safari that was causing our other uses of {{ember-infinity}} to fail. Upgrading the version of ember-in-viewport in our app (and forcing resolution via package.json) ran into the scrollableArea bug that is also fixed in this PR.

no issue
- bumps `ember-in-viewport` too `3.2.2`
- adds a fix for passing non-existent selectors to `ember-in-viewport` when there is no fixed header/footer and therefore no `.lt-scrollable` element
kevinansfield added a commit to TryGhost/Admin that referenced this pull request Mar 15, 2019
closes TryGhost/Ghost#10521
- bumped `ember-in-viewport` and `ember-infinity` which fixed the Safari bug
- forced resolution of `ember-in-viewport` to 3.2.2 to avoid conflicts across project sub-dependencies
- switched to fork of `ember-light-table` which contains a compatibility fix with `[email protected]` (PR'd here adopted-ember-addons/ember-light-table#674)
@alexander-alvarez
Copy link
Collaborator

Hey. Thanks for this. The reasoning behind the change looks 👍
Any ideas about the newly failing test?

not ok 132 Chrome 73.0 - [107 ms] - Integration | Component | lt infinity: it renders
    ---
        actual: >
            false
        expected: >
            true
        stack: >
            TypeError: Cannot read property 'destroy' of null
                at Class.destroy (http://localhost:7357/assets/vendor.js:175811:49)
                at http://localhost:7357/assets/vendor.js:19644:14
                at eachDestroyable (http://localhost:7357/assets/vendor.js:20064:9)
                at Container._Container$prototype.destroy (http://localhost:7357/assets/vendor.js:19642:5)
                at Backburner.run (http://localhost:7357/assets/vendor.js:18990:23)
                at Object.run (http://localhost:7357/assets/vendor.js:40988:27)
                at Class.willDestroy (http://localhost:7357/assets/vendor.js:22880:19)
                at Class.superWrapper [as willDestroy] (http://localhost:7357/assets/vendor.js:59170:22)
                at invokeWithOnError (http://localhost:7357/assets/vendor.js:18629:16)
                at Queue.flush (http://localhost:7357/assets/vendor.js:18688:9)
        message: >
            Cannot read property 'destroy' of null
        negative: >
            false
        Log: |
            { type: 'error',
              text: '\'TypeError: Cannot read property \\\'destroy\\\' of null\\n    at Class.destroy (http://localhost:7357/assets/vendor.js:175811:49)\\n    at http://localhost:7357/assets/vendor.js:19644:14\\n    at eachDestroyable (http://localhost:7357/assets/vendor.js:20064:9)\\n    at Container._Container$prototype.destroy (http://localhost:7357/assets/vendor.js:19642:5)\\n    at Backburner.run (http://localhost:7357/assets/vendor.js:18990:23)\\n    at Object.run (http://localhost:7357/assets/vendor.js:40988:27)\\n    at Class.willDestroy (http://localhost:7357/assets/vendor.js:22880:19)\\n    at Class.superWrapper [as willDestroy] (http://localhost:7357/assets/vendor.js:59170:22)\\n    at invokeWithOnError (http://localhost:7357/assets/vendor.js:18629:16)\\n    at Queue.flush (http://localhost:7357/assets/vendor.js:18688:9)\'\n' }

@snewcomer
Copy link
Collaborator

Could it possibly be b/c of this class syntax in this service in 2.12? https://github.com/DockYard/ember-in-viewport/blob/master/addon/services/-observer-admin.js#L16

@snewcomer
Copy link
Collaborator

Any updates here? I think if we wanted to upgrade here, we should update ember-light-table && it's travis config.

@fran-worley
Copy link
Contributor

Closing in favour of #693 @kevinansfield I've pulled this into the work their which will include bumping the in-viewport version to 3.5.5 and adding a failing test case plus fix for the non-existent selectors thing you've raised here if it is still an issue. Thanks for you contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants