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

Have m.redraw() schedule a second successive redraw when called during an existing redraw #1728

Closed
dead-claudia opened this issue Mar 20, 2017 · 16 comments
Labels
Type: Breaking Change For any feature request or suggestion that could reasonably break existing code Type: Enhancement For any feature request or suggestion that isn't a bug fix

Comments

@dead-claudia
Copy link
Member

dead-claudia commented Mar 20, 2017

Edit: Moved my previous edit lower to clarify what particularly I'm talking about.

This would entail a boolean rendering global and boolean again global within the renderer to handle this scenario. It's already come up on Gitter, and there are legitimate use cases for this.


Edit: This specific proposal coded here is junk. It should really be asynchronously scheduling something here.

Here's how it'd work, in terms of the existing API:

var redraw = m.redraw
var rendering = false
var again = false
m.redraw = function () {
    if (rendering) {
        again = true
    } else {
        do {
            again = false
            rendering = true
            redraw()
            rendering = false
        } while (again)
    }
}
@dead-claudia dead-claudia added Type: Breaking Change For any feature request or suggestion that could reasonably break existing code Type: Enhancement For any feature request or suggestion that isn't a bug fix labels Mar 20, 2017
@dead-claudia
Copy link
Member Author

Apparently, the current behavior has been tripping up people on Gitter.

@salamynder
Copy link

salamynder commented Mar 20, 2017

Problem Clarification

Following should redraw when calling m.redraw()

oncreate = (vnode) ->
    #This replaces the 'editor' div with an Ace editor
    ace.edit('#editor')
    # This redraw doesn't seem to have any effect
    m.redraw()

view = (vnode) ->
        m "#editor"

Current workaround for this is:

function asap(){
  return new Promise(function(resolve){
   setTimeout(resolve)
 })
}

// later 

function oncreate(){
  asap().then(m.redraw)
}

@cyberco
Copy link

cyberco commented Mar 20, 2017

Small typo, the asap function should be called:

function oncreate(){
  asap().then(m.redraw)
}

Although I'm not sure how the asap actually works. Why does this work?

@salamynder
Copy link

I think without asap(), mithril is still inside one redraw, not firing wished-for-redraw. With asap(), it should(?) fire after current redraw is finished. Why with oncreate hook, render not complete yet is the issue.

@dead-claudia
Copy link
Member Author

@salamynder My proposal would fix your issue with zero change from your part, BTW (although IMHO it should actually schedule something instead).

The issue you're running into is that Mithril throttles its calls, but it doesn't actually schedule anything to be run after the throttle timer ends, thus your issue.

@pygy
Copy link
Member

pygy commented Mar 21, 2017

@salamynder why do you want to redraw at that point? The initialization/redering of the editor should be independent of Mithril's redraws... Are you passing extra information to the editor on redraw? Couldn't it be passed after the ace.edit('#editor') call in oncreate instead?

If you don't mind using either the micro-task (native Promises) or the macro-task scheduler (Mithril Promise polyfill), asap becomes

function asap(){ return Promise.resolve() }

@cyberco asap().then(m.redraw) is mostly equivalent to setTimeout(m.redraw) (which is equivalent to setTimeout(m.redraw, 0)), but with then chainability. It runs m.redraw in a subsequent VM tick.

@isiahmeadows I've suggested something similar in #1592 for redraw.sync(), but with a granularity at the render level... You'd like this to work at the redraw level? How would it mesh with sync/async redraws?

@salamynder
Copy link

@cyberco
I made some tests with mithril 0.2.5 ( http://jsfiddle.net/t1z7xcf3/10/ ) and the newest mithril from unkpg.com ( http://jsfiddle.net/t1z7xcf3/22/ ). Both fiddles demonstrate the integration of Pika-calendar into input fields. Using in mithril-new the oncreate-attribute in m-helper seems to behave equally to the config-attribute of mithril-old. So from this perspective there is no problem.

@pygy
Only now, I understand that there is an oncreate-attribute. I was thinking all the shiny new lifecycle methods of components would somehow invalidate the config(+oncreate)-m-helper-hook. When you spoke of "oncreate-hook", what first sprang to my mind was component-lifecycle-oncreate-function.

@isiahmeadows
Ok, so throttling and not rescheduling is the issue. But I wonder what might be the use-case for (1) if it solves the same problem as (2): only to have oncreate packaged in a component?

Bottom-line (for me right now, just trying envisage switch to mithril-new): documentation?

However, I still don't get why there is a difference between the (1) oncreate-component-function and the (2) oncreate-m-helper-attribute, also with regard to the fact that they share the same name. This should maybe be better documented on the website, section "Resources / Integrating Third Party Stuff"? (In the vein of: http://mithril.js.org/archive/v0.2.5/integration.html ?)

@pygy
Copy link
Member

pygy commented Mar 21, 2017

@salamynder You can define hooks, both on the component and in attributes. The latter are mostly useful for defining hooks on non-component vnodes (e.g. set the focus on a given form field in oncreate or onpudate), but you can also add hooks to component with attrs.on$something()

You didn't address my previous questions, I still don't understand what you're trying to achieve and what doesn't work... Could you provide a JSBin/JSFiddle/CodePen/...? is the Ace Editor stuff a red herring and you want to redraw for other reasons?

@salamynder
Copy link

@pygy The goal for @cyberco (OP) was, I think, to render the ace editor using attrs.oncreate, but this didn't work until the asap-function was employed. Maybe @cyberco can provide a fiddle? I was spectating @cyberco 's remarks in the channel and when this ticket was opened, I tried to provide (inconclusive) context.

@pygy
Copy link
Member

pygy commented Mar 22, 2017

@salamynder my bad, I missed the original discussion on Gitter and tought you were the OP... Thanks for providing context.

@barneycarroll
Copy link
Member

@cyberco based on our conversation on Gitter, can you confirm that ACE editor initialises fine without calling an extra redraw, and that the original requirement for that redraw is redundant since ACE editor is designed to operate independently of Mithril?

@salamynder
Copy link

@pygy @cyberco @barneycarroll
To be fair, I already hijacked this issue... :) (Mainly because I didn't investigate mithril 1, yet and interest was roused by possible difference in behaviour between config/oncreate.)
So, I can as well try to bring the ticket forward a bit. This fiddle ( http://jsfiddle.net/t1z7xcf3/24/ ) is again my example of the pika-calendar, but now using Pika-component with oncreate-hook, result being that now redraw seems needed...

@dead-claudia
Copy link
Member Author

@pygy To clarify, I'm thinking of scheduling a redraw after the throttle ends.

@pygy
Copy link
Member

pygy commented Mar 24, 2017

@isiahmeadows IIRC that's +/- what happens in #1592, with a caveat when there are multiple mount points.

m.redraw() called from hooks will only re-schedule the mount points that have already been redrawn in the current cycle or the one that is being redrawn. For mount points that are yet to be redrawn, throttling kicks in (since there's still a redraw pending for them).

@dead-claudia
Copy link
Member Author

@pygy So should that be considered another reason to make m.redraw async, if it's still causing people grief?

(Or at least async when called during an existing render)

@dead-claudia
Copy link
Member Author

Closing in favor of #1847.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Breaking Change For any feature request or suggestion that could reasonably break existing code Type: Enhancement For any feature request or suggestion that isn't a bug fix
Projects
None yet
Development

No branches or pull requests

5 participants