Skip to content
This repository has been archived by the owner on Mar 8, 2022. It is now read-only.

MEGABLOB implementation #23

Merged
merged 11 commits into from
Oct 12, 2015
Merged

MEGABLOB implementation #23

merged 11 commits into from
Oct 12, 2015

Conversation

milankinen
Copy link
Contributor

I hope the code and README will explain themselves.

@staltz
Copy link
Owner

staltz commented Oct 12, 2015

screen shot 2015-10-12 at 10 24 37

I was able to put the application in this invalid state, it wouldn't load data anymore.

@milankinen
Copy link
Contributor Author

What should happen if all siths are scrolled out? Or should this be prevented somehow?

@jas-chen
Copy link
Contributor

Take a look at #12

@milankinen
Copy link
Contributor Author

Thanks! Scroll preventing implemented.

It would be awesome if that "rule" could added to the README so that others can be prepared before making any PRs.

@staltz
Copy link
Owner

staltz commented Oct 12, 2015

It's not a rule ;)

@milankinen
Copy link
Contributor Author

Anyhow, should be fix'd now

@staltz
Copy link
Owner

staltz commented Oct 12, 2015

Why does it take more than a second for a previously-canceled request to get resent again? E.g. try pressing quickly down, up, down. The relevant request should be sent immediately, I don't understand why is there an idle time.

@milankinen
Copy link
Contributor Author

My mistake... sry. It was because I was lazy and sent the cancel events every time when the scroll was pressed. :-) However, the ordering of the emitted events from sithsScrolledS and appState.changes() can't be guaranteed.

In this case, cancel events (from sithsScrolledS) were emitted after the load master/apprentice events (from appState.changes()), resulting that no ajax request was sent after scrolling... until planet was changed (which caused new event to appState.changes()).

Now sending events only from appState.changes() (should have done this from the very beginning...) so the order of the emitted cancels/loads is guaranteed.

@staltz
Copy link
Owner

staltz commented Oct 12, 2015

This works. Thanks!

staltz added a commit that referenced this pull request Oct 12, 2015
@staltz staltz merged commit aed7cd6 into staltz:master Oct 12, 2015
@aparticka
Copy link

This one's my favorite, I need to start using Bacon.

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

Successfully merging this pull request may close these issues.

None yet

4 participants