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

Forcedraw #198

Merged
merged 6 commits into from
Aug 16, 2014
Merged

Forcedraw #198

merged 6 commits into from
Aug 16, 2014

Conversation

Zolmeister
Copy link
Contributor

Added optional param to m.redraw(true) to allow forced redrawing, overriding default asyncronicity

This is built off of #197

@lhorie
Copy link
Member

lhorie commented Aug 15, 2014

The only reservation I have with this PR is that it makes the .then(m.redraw) idiom a bit more expensive in edge cases where two or more requests finish within the span of one frame.

This idiom is useful for showing loading icons, for example.

This can probably be mitigated for 99.9% of the cases by doing a strict comparison (i.e. === true) instead of !force

@lhorie
Copy link
Member

lhorie commented Aug 15, 2014

Oh, and it's failing the CI build for some reason, which is preventing me from merging it

@Zolmeister
Copy link
Contributor Author

@lhorie Sent you an email, this PR depends on #197
If you're willing to merge the whole thing, I'll resolve conflicts to let you merge cleanly (let me know)
and also make it === true

@lhorie
Copy link
Member

lhorie commented Aug 15, 2014

Yep, I saw it. I just added you as collaborator.

It's a bit hard to read the diff, but I think this change can be merged without bringing in #197.

Conflicts:
	archive/v0.1.20/mithril-tests.js
Zolmeister added a commit that referenced this pull request Aug 16, 2014
@Zolmeister Zolmeister merged commit fd81f06 into MithrilJS:next Aug 16, 2014
@Zolmeister Zolmeister deleted the forcedraw branch August 16, 2014 19:46
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.

2 participants