paf31 / purescript-jquery

Type declarations for jQuery
MIT License
30 stars 28 forks source link

Add display (jQuery's show) and hide #17

Closed anttih closed 9 years ago

garyb commented 9 years ago

Could perhaps use toggle with a boolean parameter for this instead? Still avoids the unfortunate show name collision, and is just as easy to use thanks to currying: toggle true instead of display, toggle false instead of hide.

anttih commented 9 years ago

@garyb Agreed. I pushed the change. Thanks!

paf31 commented 9 years ago

I'm going to be a pain, and disagree here. I think toggle should be reserved for the JS call toggle() which switches the current state from shown to hidden and vice versa. I think toggle' or something makes sense for this version, but I also think that show and hide would be useful, even if they are just aliases for toggle' true and toggle' false.

paf31 commented 9 years ago

That said, I don't have a solution for the name collision problem.

anttih commented 9 years ago

That makes sense too. I think display is ok since what it really does is change the display css property.

garyb commented 9 years ago

Fair enough, personally I don't ever use toggle without passing the parameter, as the implicit state usually becomes a problem later on, but if you think it's better to stick the the "default" behaviour that does make sense.

paf31 commented 9 years ago

I see what you're saying, but I think of the DOM effect as encompassing a whole bunch of other effects, including state.

garyb commented 9 years ago

Maybe we use toggle for the default and display(bool) for the parameterised version? Although that doesn't help with the name collision thing again if we want show/hide equivalents.

joneshf commented 9 years ago

toggle does that in jquery: http://api.jquery.com/toggle/#toggle-showOrHide

Like most jquery stuff it has multiple different behaviors depending on what arguments are passed to it.

anttih commented 9 years ago

I added toggle as toggle(). I think either: keep show as display() or rename to something else like unhide. But I do like having separate show/hide functions.

paf31 commented 9 years ago

@anttih Thanks, this looks good.

Also, we always have import hiding and import qualified if we really want to use show. Just saying, it's an option...

anttih commented 9 years ago

Can we get this in, or was there something still missing?