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

Leverage ember-native-dom-helpers for better interaction testing #57

Merged
merged 3 commits into from
Mar 3, 2017

Conversation

alexander-alvarez
Copy link
Collaborator

@alexander-alvarez alexander-alvarez commented Feb 24, 2017

Also improved the weird naming surrounding the first argument to jumpTo and added docs.

related:
@cibernox -- I couldn't get the equivalent of this.$(elem).scrollTop() or .scrollLeft() to work.
It seems like scrollTop() simply modifies the scrollTop property on the elment, which generates an async event that doesn't contain much info about the scroll, besides that it did (and the target element, that has the mutated property already). I haven't been able to get my tests to wait for the scroll event resolve and callback, etc.

because of this we have some nastiness in the form of arbitrary delay times.. https://github.com/alphasights/ember-scrollable/blob/master/tests/integration/components/scroll-content-element-test.js#L122

Dev console images of scroll event

image

image

@alexander-alvarez
Copy link
Collaborator Author

cc @pixelhandler might have some thoughts too?

@pixelhandler
Copy link

@alexander-alvarez here is the jQuery implementation, https://github.com/jquery/jquery/blob/bf3a43eff8682b59cec785be6003753fa4b93706/src/offset.js#L174-L203 and MDN docs, https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollTop

Does (HTMLElement) el.scrollTop and el.scrollTop = value work? Using the find helper you can still wrap with jQuery if you need to, e.g. jQuery(find('.it'))

@alexander-alvarez
Copy link
Collaborator Author

@pixelhandler I get that, and that will dispatch the event, unfortunately we have a bigger problem.

which is scroll events are asynchronous, don't bubble, and their's no way to get listeners of an element that has been registered natively with addEventListener (like is exposed to the devtools: getEventListenersObject).

This means that there's no nice way to do the following with basic html elements

// element with scroll callback
trigger(el, 'scroll', { // data })
waitFor(el, 'scroll');
// assert updated state

I can get around this limitation for this repository because we use ember lifeline which keeps track of it's listeners (and uses jQuery, so we could even fallback on _data), but it would be nice if we have a way to do this with any ember project. And if I were trying to test scrolling in my app and was having issues I would first go to the ember-scrollable repo to see how they test it -- and I would be horrified to see https://github.com/alphasights/ember-scrollable/blob/master/tests/integration/components/scroll-content-element-test.js#L133..L149

That being said I'm going to get rid of this ^ ugliness, using the ember-lifeline APIs, and if someone gets the same issues they can follow in my footsteps.

@cibernox
Copy link

cibernox commented Mar 3, 2017

Hey. I haven't followed this conversation closely, but I think that now I get it. I didn't know that scroll events were asynchronous unlike the rest.

I think that the changes proposed in cibernox/ember-native-dom-helpers#29 can help.

The tl;dr; is that find/findAll will be async. They would poll the DOM for changes and after a few tries if all waiters have settled then it should give up.
You can probably trigger a scroll and search for the element you expect to appear when the scroll action is fired.

Soon.

@alexander-alvarez
Copy link
Collaborator Author

@cibernox yup -- you get it.

Wouldn't that just replacing my arbitrary delay code with their arbitrary delay code ?

Is that "good enough ™️" ?

Or should we find a way to hook into those callbacks during testing -- I opened up ember-lifeline/ember-lifeline#19 in an attempt to spur conversation (since we're using ember-lifeline in this repo)

@cibernox
Copy link

cibernox commented Mar 3, 2017

I'd say that both solutions are more or less equivalent, yes. The syntax would be nicer with an async find.

scroll({ x: 25, y: 25 });

assert.ok(await find('.loading-spinner'), 'A spinner is shown while infinitescroll loads more entries');
assert.ok(await find('.new-content'), 'New content has appeared');

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.

None yet

4 participants