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

Unify _input/_change names #22

Closed
mightybyte opened this issue Aug 31, 2015 · 13 comments
Closed

Unify _input/_change names #22

mightybyte opened this issue Aug 31, 2015 · 13 comments
Labels

Comments

@mightybyte
Copy link
Member

The TextInput and TextArea types have _input events, but with Checkbox and Dropdown they're called _change. This terminology difference confused me, so I think it should probably be unified. I personally like _change better but don't have a huge attachment to that name. It seems like _input is ok for everything except _textInput_input where the two inputs made me fail to notice it all on the first pass.

@spl
Copy link
Contributor

spl commented Sep 1, 2015

I think these field names are based on the DOM event names themselves: input for <input> and <textarea> and change for <select>. While that doesn't make it less confusing, it's at least consistent with the DOM naming scheme. I do think documenting the connection to the DOM events for these fields would help, at the very least.

@mightybyte
Copy link
Member Author

@spl If that's the case, it still seem strange because all those elements have both of those events.

@spl
Copy link
Contributor

spl commented Sep 1, 2015

@mightybyte No argument there. I'm guessing these options were chosen for their immediate/common usefulness rather than trying to support all available event types, since they are not all equally useful.

Of course, it would be nice to easily use any DOM event on any widget. I've run into this problem myself.

@mightybyte
Copy link
Member Author

Yeah. I'm playing with some different ideas for building widget infrastructure in the reflex-dom-contrib package. I'm currently just building on top of reflex-dom widgets, but I certainly don't have to. If you want to play with API ideas for supporting all DOM event types, I'll definitely accept those changes in that repo. To start with you should probably make a separate module to avoid breaking backwards compatibility, but eventually we can merge all these ideas together.

@spl
Copy link
Contributor

spl commented Sep 2, 2015

@mightybyte I looked at it briefly, and it looks nice! I did borrow the useful widgetHoldHelper from reflex-dom-contrib.

As for the input widgets, I've found that it's important to be careful about the source and flow of events to avoid cycles. For example, use _dropdown_change instead of _dropdown_value.

It would be nice if all widgets naturally didn't allow for event cycles. That is, I'd like the value of the input when it actually changes in the DOM, and whether that change be due to a programmatic update or a user input shouldn't matter, but my program shouldn't loop infinitely if I tie the widget's value (indirectly) to the widget's setEvent. I'm not sure if that is possible, but I'd naively like to think so.

@oliver-batchelor
Copy link
Collaborator

It is possible to automatically stop the loop using value equality - e.g.
using nubDyn. However it's a bit hacky, what if the user sets the value
twice to the same thing (you might want to know about that)? Maybe better
would be some kind of loop-detection error in the core when you stuff
things up.

I also started working on this previously and I think we can avoid a lot of
boilerplate by not using so many data types. Just like @mightybyte's
reflex-dom-contrib I think we can get away with using just one data type
most of the time, they're pretty much all have a value which can be changed
by a user, or progmatically and have a value of some type.

Btw. @mightybyte - does the cool switchable widget trick work over the new
event fan?

FWIW here was my attempt:
https://github.com/Saulzar/reflex-html/blob/apphost/src/Reflex/Html/Input.hs

On Wed, Sep 2, 2015 at 6:07 PM, Sean Leather notifications@github.com
wrote:

@mightybyte https://github.com/mightybyte I looked at it briefly, and
it looks nice! I did borrow the useful widgetHoldHelper from
reflex-dom-contrib.

As for the input widgets, I've found that it's important to be careful
about the source and flow of events to avoid cycles. For example, use
_dropdown_change instead of _dropdown_value.

It would be nice if all widgets naturally didn't allow for event cycles.
That is, I'd like the value of the input when it actually changes in the
DOM, and whether that change be due to a programmatic update or a user
input shouldn't matter, but my program shouldn't loop infinitely if I tie
the widget's value (indirectly) to the widget's setEvent. I'm not sure if
that is possible, but I'd naively like to think so.


Reply to this email directly or view it on GitHub
#22 (comment)
.

@spl
Copy link
Contributor

spl commented Sep 2, 2015

@Saulzar I'm not sure how to stop the loop using nubDyn. My naive impression was that the problems were due to event trigger cycles, which happened regardless of the value of the event. But I'd love to understand better how cycles happen and how to detect what is causing them.

Also, wrt the ideas about input widgets, I think it is useful to keep in mind that the DOM event types are not a closed world. New event types can be created and handled. I've seen some JQuery-based libraries that do this. From that perspective, it would be nice if one could easily register a Reflex-based event handler for an arbitrary DOM event. I did this, but it required a massive hack using a GHCJS JQuery wrapper to listen for the event and call a Reflex handler. (This was challenging because (1) ghcjs-dom functions didn't do anything in response to the event – even though I could add a listener in JS – and (2) ghcjs-jquery didn't work for me – see ghcjs/ghcjs-jquery#10.)

@oliver-batchelor
Copy link
Collaborator

Actually no - you're right. I think there's nothing you can do if you have
a direct cycle! Then runtime could possibly detect a cycle however, and
give an error - that should be possible, since Events should never fire
twice in the same frame.

You should be able to make a wrapper to register a listener for arbitrary
events (by name?), it's just a callback handler from JS after all right?
Even if there isn't a simple function to do it right now (fully agree it
should be simple). It should exist somewhere on the Element API though -
where the rest of the event types are.

I'm not really familiar with jquery at all, so I have no idea how that
relates - maybe one of the more web-developer-y kinds around here knows
something :)

On Wed, Sep 2, 2015 at 9:25 PM, Sean Leather notifications@github.com
wrote:

@Saulzar https://github.com/Saulzar I'm not sure how to stop the loop
using nubDyn. My naive impression was that the problems were due to event
trigger cycles, which happened regardless of the value of the event. But
I'd love to understand better how cycles happen and how to detect what is
causing them.

Also, wrt the ideas about input widgets, I think it is useful to keep in
mind that the DOM event types are not a closed world. New event types can
be created and handled. I've seen some JQuery-based libraries that do this.
From that perspective, it would be nice if one could easily register a
Reflex-based event handler for an arbitrary DOM event. I did this, but it
required a massive hack using a GHCJS JQuery wrapper to listen for the
event and call a Reflex handler. (This was challenging because (1)
ghcjs-dom functions didn't do anything in response to the event – even
though I could add a listener in JS – and (2) ghcjs-jquery didn't work
for me – see ghcjs/ghcjs-jquery#10
ghcjs/ghcjs-jquery#10.)


Reply to this email directly or view it on GitHub
#22 (comment)
.

@spl
Copy link
Contributor

spl commented Sep 2, 2015

It should exist somewhere on the Element API though - where the rest of the event types are.

It does, and it didn't work. ;)

@spl
Copy link
Contributor

spl commented Sep 2, 2015

I'm not really familiar with jquery at all, so I have no idea how that relates - maybe one of the more web-developer-y kinds around here knows something :)

Just to clarify, what I'm describing is not a JQuery-specific thing. You can trigger and listen to arbitrarily-named event types in plain old JavaScript. It's just that I observed it happening in JQuery libraries.

@oliver-batchelor
Copy link
Collaborator

Thinking it's probably something to do with the way the JS lib is loaded -
GHCJS.DOM is a pretty thin wrapper, it's not like it does any actual event
processing itself.

As always - feel free to provide a small example which doesn't work :)

On Wed, Sep 2, 2015 at 10:22 PM, Sean Leather notifications@github.com
wrote:

I'm not really familiar with jquery at all, so I have no idea how that
relates - maybe one of the more web-developer-y kinds around here knows
something :)

Just to clarify, what I'm describing is not a JQuery-specific thing. You
can trigger and listen to arbitrarily-named event types in plain old
JavaScript. It's just that I observed it happening in JQuery libraries.


Reply to this email directly or view it on GitHub
#22 (comment)
.

@mightybyte
Copy link
Member Author

@spl Yes, the _change events are a very important tool for eliminating event loops. There are ways to do it without them, but using them can make things quite a bit simpler.

@Saulzar I haven't tried the switchable widget with fan, so I'm not sure.

@ryantrinkle
Copy link
Member

Hi everyone,

This has been an interesting discussion to follow! A lot of valid concerns have been raised, and I hope to be able to address them soon.

Regarding non-custom DOM events, I've begun putting in place a system that allows us to support all of them, without paying for the ones we don't use: https://github.com/ryantrinkle/reflex-dom/blob/master/src/Reflex/Dom/Widget/Basic.hs#L720 . This domEvent function allows you to say something like domEvent Click to get the click event. I'll be applying this approach to other widgets soon, too, which I think will improve things.

The remaining issue with supporting all built-in DOM events is that it is not quite clear to me what should be marshaled for each DOM event type, and whether we can do it lazily to avoid making users who don't care about the details of common events pay for marshaling a bunch of extra metadata. I'd very much welcome suggestions on this.

Regarding cycles, I think that separation between change/input events and updated values will always be necessary to a degree - some logic does depend on whether the user made the change or the software made the change. (I completely agree, by the way, that having "change" and "input" be distinct is very confusing, but unfortunately it is the way the DOM works - the new uniform approach to events may actually make things worse in this regard, because "the wrong" event will now be available.) However, I would be very supportive of any concrete proposals that would decrease the chance of users running into cycles or improve debugging for ones that exist.

Regarding custom DOM events, I have not looked into this, but I'd be happy to hear suggestions on what this interface should look like from someone who has.

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

No branches or pull requests

6 participants