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

Remove jQuery, use flatpickr instead of pikaday #842

Merged
merged 8 commits into from
Aug 6, 2018

Conversation

RobbieTheWagner
Copy link
Member

ember-pikaday was using jQuery internally, so we had to move away from it. I chose flatpickr, but we could use anything else. I'm having trouble getting the tests to pass currently.

Resolves #590

@RobbieTheWagner
Copy link
Member Author

@teddyzeenny the one test failure I am getting here is related to https://github.com/emberjs/ember-inspector/blob/master/tests/acceptance/object-inspector-test.js#L564.

not ok 37 Chrome 67.0 - Object Inspector: Date fields are editable
    ---
        actual: >
            view:getTree
        expected: >
            objectInspector:saveProperty
        stack: >

It seems objectInspector:saveProperty is not being called, although it does seem to save the new dates when I manually click through it. I was thinking maybe fillIn does not trigger the same things on flatpickr as it did on pikaday. Any ideas?

ember-pikaday was using jQuery internally, so we had to move away from it. I chose flatpickr, but we could use anything else. I'm having trouble getting the tests to pass currently.

Resolves #590
@RobbieTheWagner RobbieTheWagner force-pushed the remove-jquery-use-flatpickr branch from 3d18854 to f4f6170 Compare July 27, 2018 03:52
@RobbieTheWagner RobbieTheWagner requested a review from acorncom July 27, 2018 03:53
@RobbieTheWagner RobbieTheWagner requested a review from rwjblue August 6, 2018 16:21
@nummi
Copy link
Collaborator

nummi commented Aug 6, 2018

Have you tried calling other events manually? I see a keydown — what about keyup or blur?

@RobbieTheWagner
Copy link
Member Author

@nummi I fixed that, everything works, I just need someone to review the PR.

@Bestra
Copy link
Contributor

Bestra commented Aug 6, 2018

I'll review

Copy link
Contributor

@Bestra Bestra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried this locally and it looks like flatpickr is functioning as intended. I loaded up http://ember-flatpickr.shipshape.io/ and was able to inspect the dates on the date picker component using it. Hitting 'esc' closed the picker as expected. 👍

screen shot 2018-08-06 at 5 27 19 pm

@RobbieTheWagner RobbieTheWagner merged commit 1c4a832 into master Aug 6, 2018
@RobbieTheWagner RobbieTheWagner deleted the remove-jquery-use-flatpickr branch August 6, 2018 22:53
teddyzeenny pushed a commit that referenced this pull request Aug 13, 2018
* Remove jQuery, use flatpickr instead of pikaday

ember-pikaday was using jQuery internally, so we had to move away from it. I chose flatpickr, but we could use anything else. I'm having trouble getting the tests to pass currently.

Resolves #590

* Fix test

* Check ember version before removing jQuery

Co-Authored-By: Robert Jackson <[email protected]>

* Stop trying to use ember-native-dom-dispatcher

* Remove unused project

* Add travis_retry to attempt to retry builds automatically
cyril-sf pushed a commit to cyril-sf/ember-inspector that referenced this pull request Mar 30, 2022
* Remove jQuery, use flatpickr instead of pikaday

ember-pikaday was using jQuery internally, so we had to move away from it. I chose flatpickr, but we could use anything else. I'm having trouble getting the tests to pass currently.

Resolves emberjs#590

* Fix test

* Check ember version before removing jQuery

Co-Authored-By: Robert Jackson <[email protected]>

* Stop trying to use ember-native-dom-dispatcher

* Remove unused project

* Add travis_retry to attempt to retry builds automatically
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants