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

ES6 syntax #17

Open
petermakowski opened this issue May 8, 2017 · 7 comments
Open

ES6 syntax #17

petermakowski opened this issue May 8, 2017 · 7 comments
Assignees

Comments

@petermakowski
Copy link
Collaborator

Hey @d4nyll, I am going to use lethargy in my project (thanks again, you're a life saver) and have started converting it to ES6 syntax (it was the only option due to my current setup). I was wondering what are your plans regarding the library, as I know you plan to refactor https://github.com/d4nyll/smartscroll to ES6 at some point.

I will gladly share my work here (translate to ES6 and add Babel to transpile to ES5) if you're willing to move away from coffee script. Please let me know your thoughts.

@d4nyll
Copy link
Owner

d4nyll commented May 8, 2017

Hi @petermakowski I would definitely appreciate it if you can port it to ES6. The only reason I used CoffeeScript was because it supported classes when ES6 was not so popular.

I'll add you as a collaborator on this project, which means you can create a new branch on this repository and push your work on it, without having to fork it and do pull request.

@d4nyll
Copy link
Owner

d4nyll commented May 12, 2017

Hey @petermakowski I see you've ported the code using decaffeinate. Do you mind if I carry on with it and refactor it some more? I got a few people who are available at work to refactor it as an exercise.

@petermakowski
Copy link
Collaborator Author

Absolutely, feel free to do so - I'm not 100% happy with how it looks yet, your original coffeescript was much cleaner and minified JavaScript was smaller. I not only converted it using decaffeinate but fixed multiple issues in the code manually as well. I planned to improve it more, but I'm busy with other projects at the moment. I was thinking of using Array map for coffeescript ranges, but that would need testing as apparently it's slower (havent' tried it myself) - https://github.com/hemanth/coffeescript-equivalents-in-es6#ranges

@d4nyll
Copy link
Owner

d4nyll commented May 12, 2017

We've made a few changes, I'll clean it up a little and push it on Monday.

@d4nyll d4nyll removed their assignment May 18, 2017
@petermakowski
Copy link
Collaborator Author

petermakowski commented May 18, 2017

Hey, I made quite a few changes, please review (look at the commit message). Could you let me know what else would you need before merging to the master branch?

There is definitely still some bugfixing to do as I noticed that since this commit 58382fc Firefox keeps returning false after first 2 scrolls no matter what you do (tested with Macbook's trackpad).

I can look into it if you'd like.

@d4nyll
Copy link
Owner

d4nyll commented May 18, 2017

There were some changes I wanted to make to the code, such as making the defaults available as a static property of the class. I'll make those changes over the weekend.

After that, you can take a final look and we can then test it on different browsers at the beginning of next week, fix any bugs and merge it into master next weekend.

How's that sound?

@petermakowski
Copy link
Collaborator Author

Sounds good! Actually I managed to fix Firefox issue just now 332fe29.

@petermakowski petermakowski changed the title ES6 syntax and ES6 modules ES6 syntax May 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants