-
Notifications
You must be signed in to change notification settings - Fork 33
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
One Way Data flow & Two Scrollbars #29
Conversation
…be event listeners on the element so we can drag outside of the scroll area, and have it update
I just wanted to let you know that I started looking at the code and it's looking great so far. I haven't had a chance to pull it down yet and take a deeper look. |
Thanks @taras . Just got the two scrollbars to show up and trigger appropriate scroll events.
I decided I liked the first approach. I'm thinking |
out of curiousity -- you wouldn't happen to know if this file is needed: https://github.com/alphasights/ember-scrollable/blob/master/blueprints/ember-scrollable/index.js it's the only piece of code I see that references trackpad scroll emulator |
I'm not sure if it's necessary because it's now added as a dependency. I
don't remember, it's been a while. My only concern would be whether it
would work in ember-light-table without that code.
…--
Taras Mankovski
On December 9, 2016 at 9:42:50 AM, Alex Alvarez ***@***.***) wrote:
out of curiousity -- you wouldn't happen to know if this file is needed:
https://github.com/alphasights/ember-scrollable/blob/master/blueprints/ember-scrollable/index.js
it's the only piece of code I see that references trackpad scroll emulator
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#29 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAEjvwUUdXR8YTdcrwkwbUTLJEJmGxsEks5rGNVCgaJpZM4LG6nD>
.
|
Planned tests (covered by integration tests of the elements
Double Scrollbar Tests (Acceptance tests)
|
const secondPosition = 25; | ||
|
||
// WHEN the scrollY position has moved 10 px | ||
this.$(cssSelector).scrollTop(firstPosition); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@taras I was struggling to get these badboys triggering the scroll events synchronously.
the source binding the scroll is as such: this.$().on('scroll', bind(this, this.scrolled));
I tried wrapping these blocks in run
, run.next
etc, to no avail.
any experience with this?
…oper mouse x and y positions
@alexander-alvarez that's fine. I would also add a deprecation in the |
@alexander-alvarez is this ready to be merged? |
@offirgolan almost...
|
Why do you want to adjust zoom programmatically?
We could use |
@taras adjust for the zoom programatically. I want a variable instead of right now we're doing bind , which I guess I can replace the with the code from https://github.com/rwjblue/ember-lifeline#addeventlistener if that's the best practice and will let me replace the arbitrary run.later's with run loops. I don't think requiring 2.6+ is a little excessive at the moment, no? |
I'm not sure how you'd handle that. We could remove zoom from the test suite. What do you think?
I'm not sure. I've seen |
Any particular reason why it's there? Where is this even done? oh wait.. I already added |
It's part of the default app blueprint. It's in |
@alexander-alvarez can't we just force the zoom to be <style>
#ember-testing-contaner {
zoom: 1 !important;
}
</style> Note: I'm not sure if the id of the container is correct, just going off the top of my head here. |
tried write a test helper, that blocks for the scroll events which are asynchronous, but still not passing -- I'm pretty sure it's related to the scroll events triggered by |
Conflicts: addon/components/ember-scrollable.js
@alexander-alvarez are these test being run on chrome or phantom? |
@offirgolan travis it seems they're using chrome (on my local it's with chrome) |
What needs to be done to finalize this? Test is failing because of: |
5fe5dbb
to
851f87d
Compare
851f87d
to
ed0f3be
Compare
I updated to follow the LTS releases supported by |
Looks like test are passing now as well. Awesome work @alexander-alvarez. cc @taras. Lets get this baby in so we can start figuring out how to integrate this with ELT. |
Amazing job here guys!!! |
* alphasights/master: One Way Data flow & Two Scrollbars (alphasights#29) Remove QUnit zoom (alphasights#38)
I gutted the
ember-scrollable
class in favor of a more DDAU approach. Allowing me to make the double scroll easily.This made it easier to set up listeners on the appropriate 'information-hierarchy' level and delegate functionality to different components
new components (ideally would be private):
scroll-content-element
-- Handles scroll events within the body of the scroll content, properly sets scrollTop / scrollLeft property depending on the configuration and the givenscrollTo
.ember-scrollbar
-- Handles displaying and moving the handle within the confines of it's template, and has callbacks for intending to dragging and jump to particular positions.TODO
Extend api to allow for left and right scroll & implementFix JiterrynessLots of documentationTestsConsiderations
End goal is something like this (except for ours hides when the user isn't moving the mouse in the scroll area):
http://automattic.github.io/antiscroll/