Skip to content

Instantly share code, notes, and snippets.

@rtfeldman
Last active January 14, 2017 16:13
Show Gist options
  • Star 4 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save rtfeldman/5f015adbdfbba541c7e7e1409b6efeef to your computer and use it in GitHub Desktop.
Save rtfeldman/5f015adbdfbba541c7e7e1409b6efeef to your computer and use it in GitHub Desktop.
{-| DOM
## Focus
@docs focus, blur
## Measurement
@docs measureText, bounds, Bounds
## Scrolling
@docs getScrollTop, setScrollTop, getScrollLeft, setScrollLeft
## Selecting
@docs SelectorError
-}
{-| After the next render, runs [`document.querySelector`](https://developer.mozilla.org/en-US/docs/Web/API/Document/querySelector)
on the given selector and [sets focus](https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/focus) on the
resulting element.
Html.focus "#searchInput"
NOTE: setting focus can silently fail if the element is invisible. This could be captured as an error by checking to see
if document.activeElement actually got updated to the element we selected. https://jsbin.com/xeletez/edit?html,js,output
-}
focus : String -> Task SelectorError a
{-| After the next render, runs [`document.querySelector`](https://developer.mozilla.org/en-US/docs/Web/API/Document/querySelector)
on the given selector and [removes focus](https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/blur) from the
element it finds.
Dom.blur "#searchInput"
-}
blur : String -> Task SelectorError a
{-| The [`scrollTop`](https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollTop) of the element returned
by the given [`querySelector`](https://developer.mozilla.org/en-US/docs/Web/API/Document/querySelector).
-}
getScrollTop : String -> Task SelectorError Float
{-| Set the [`scrollTop`](https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollTop) of the element returned
by the given [`querySelector`](https://developer.mozilla.org/en-US/docs/Web/API/Document/querySelector).
-}
setScrollTop : String -> Float -> Task SelectorError Float
{-| The [`scrollLeft`](https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollLeft) of the element returned
by the given selector.
-}
getScrollLeft : String -> Task SelectorError Float
{-| Set the [`scrollLeft`](https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollLeft) of the element returned
by the given selector.
-}
setScrollLeft : String -> Float -> Task SelectorError Float
{- Find out how much space styled text takes up. You can provide an optional
fixed width.
-}
measureText : List ( String, String ) -> Maybe Float -> String -> ( Float, Float )
{- Calls [`getBoundingClientRect()`](https://developer.mozilla.org/en-US/docs/Web/API/Element/getBoundingClientRect) on the element returned
by the given [`querySelector`](https://developer.mozilla.org/en-US/docs/Web/API/Document/querySelector).
-}
bounds : String -> Task SelectorError Bounds
type alias Bounds =
{ top : Float, bottom : Float, left : Float, right : Float }
{- Describes the result of a failed call to [`document.querySelector`](https://developer.mozilla.org/en-US/docs/Web/API/Document/querySelector).
`InvalidSelector` represents a [`SYNTAX_ERR` exception](https://developer.mozilla.org/en-US/docs/Web/API/Document/querySelector#Notes).
`ElementNotFound` represents the case where `document.querySelector` did not find an element.
-}
type SelectorError
= InvalidSelector String
| ElementNotFound
@rtfeldman
Copy link
Author

@gampleman Yep, agree both that it'd be nice and that it isn't worth it for the time being. 😄

@rtfeldman
Copy link
Author

I will not be available for a week and this API probably will be implemented before then.

Oh I don't expect that! We're still in the "figuring out what makes sense" phase...implementation is a ways off yet. 😉

For this you need to compare current items bounds with the current mouse position to determine if it will go above (top third), inside (middle third) or below (bottom third) of the "hovered" item.

Hm...wouldn't this work?

  1. On mousedown, run a Task for each item to record its bounds in the Model, then record dragging = True or the like
  2. On mousemove decode the current mouse position and send a Msg to update with the new mouse position
  3. In update, use the bounds in the Model and the received mouse coordinates to do this calculation

In all the drag-and-drop stuff I've done, neither the bounds of the dragged item (including the placeholder) nor the items below ever change on mousemove, so recording them on mousedown seems like it would do the trick.

Please correct me if I'm missing something!

@gdotdesign
Copy link

So right now my implementation is working like this:

  1. on mousedown set dragging = True, draggedLayer = id, dummyPosition = (Below, id)
  2. on mousemove of every layer the dimensions of that layer, the mouse position are decoded and used to figure out where the dragged layer will end up and set dummyPosition accordingly. During rendering the position of dragged item is shown with a new element that is inserted in the DOM.
  3. on Mouse.ups the drag stops and the layer get rendered in it's new place

There three use cases I can think of right now where caching the bounds would result in an invalid cache:

  • In almost all the cases I've dealt with drag and drop the list / tree is scrollable and if it is big and the mouse gets to the edge then it is scrolled programmatically to show more items and by doing so all of the items bounds change.
  • In a tree if a branch is opened or closed
  • In many cases the dragged elements dummy object (which shows where the dropped element will be) is the same size as the dropped element. So moving the dragged element will move the other elements and make their bounds change.

Here is a video of the typical dragging interaction that I needed to create (and which I created) https://youtu.be/wWl6sICdlCA

@rtfeldman
Copy link
Author

Thanks for following up @gdotdesign! Some thoughts:

In almost all the cases I've dealt with drag and drop the list / tree is scrollable and if it is big and the mouse gets to the edge then it is scrolled programmatically to show more items and by doing so all of the items bounds change.

If you're programmatically scrolling down by x, you can programmatically move all their stored bounds down by x as well, yeah?

In a tree if a branch is opened or closed

This seems like it has the same characteristics as mousedown - that is, it can be covered by a Task as far as I can tell...

In many cases the dragged elements dummy object (which shows where the dropped element will be) is the same size as the dropped element. So moving the dragged element will move the other elements and make their bounds change.

Sure, but when moving things mid-flight, you're moving things whose sizes are all in the model, and after dropping, you can re-measure via a Task, yeah?

Just to reiterate, the goal is to keep the API as small as possible. The central question I'm trying to answer is "can this be done without race conditions using just the function that returns a Task?" So far it still seems like the answer is "yes," right?

@debois
Copy link

debois commented Jul 18, 2016

Its great this is finally happening, many thanks!

About the functionality.

I think what you've got covers all I've met in so far elm-mdl, so it'd make me very happy. That said, I'll need eventually the ability to work with selections on input elements. I'd like to propose (satisfying all your 3 requirements from the OP the following addition:

{-| After the next render, runs [`document.querySelector`](https://developer.mozilla.org/en-US/docs/Web/API/Document/querySelector) 
on the given selector and [sets the selection](https://developer.mozilla.org/en-US/docs/Web/API/HTMLInputElement/setSelectionRange) 
on the resulting element if they are input nodes, or fails. 

    Html.setSelectionRange "#searchInput" 2 5 Nothing 
-}
setSelectionRange : String -> Int -> Int -> Maybe Bool -> Task SelectError a

About the API.

  1. It's not clear to me why the result type are Task for all these. I may be mistaken, but as a user, won't I always want to use these Tasks by converting them to Cmd? If that's the case, then perhaps an API based on Cmd and Result rather than Task would me more convenient for everybody and easier to digest for beginners:

    setScrollTop : String -> Float -> a -> Cmd (Result SelectorError a)

  2. I think a very common use will be to use the setter APIs, say setScrollTop, then ignore any selection errors. (We could have a long philosophical debate about whether getting the selector wrong should be a run-time error; I hope y'all'll agree that at least when prototyping, I can reasonably just watch it fail if I get it wrong.) Catering to the common case of not caring about/handling selection errors, maybe each function could be provided in two variants:

    setScrollTopWithErrors : String -> Float -> a -> Cmd (Result SelectorError a)
    setScrollTop : String -> Float -> a -> Cmd a
    

Personally, I'd be using the latter variant every time. (Well, at least until my program starts breaking in weird ways, but then I have the option of using the former variant.)

Comments outside the rules

This comment is out-of-bounds because it ignores the rule that we can't introduce dependencies on virtualdom. However, I think that Elm sometimes under-emphasises programmer convenience, and that this API is a step in that direction, so I feel the need to say this anyway:

The presently proposed API falls somewhat short of the convenience that Elm otherwise offers for working with the DOM. E.g., setting CSS properties of a div—I can do it in one line:

div [ style ("width", "256px") ] [ ... ]

In contrast, if I wish to set scrollTop of the same div, I need to

  1. Figure out in update (as opposed to view) that I need to set scrollTop
  2. Emit a special Cmd setting scrollTop.
  3. Do nothing when that Cmd comes back to me.

What I could reasonably hope for, considering the remainder of the Elm HTML API, was to simply be able to do this:

div [ scrollTop 256 ] [ ... ] 

This approach—which obviously requires messing with virtualdom, implementation-wise—would allow me to keep view-functionality in the view function and avoid a needless call to update when the Cmd returns.

As I said, I understand this is not on the table right now, but I do think the syntactical and cognitive overhead of the currently proposed API should be revisited at some point later, especially when such a nice alternative API seems within reach.

@fredcy
Copy link

fredcy commented Jul 18, 2016

The list of features looks good to me.

In the code, getScrollTop and setScrollTop each appear twice. Looks like the latter should be getScrollLeft and setScrollLeft.

@Gozala
Copy link

Gozala commented Jul 18, 2016

Hi @rtfeldman I'm glad that focus management issues have being looked at. I would like to raise some concerns on the proposed API though. In fact I've being using exactly same API for sometime now and have being switching to an alternative due to following issue:

CSS selectors are used as a means to identification on which UI element task should be performed. Major disadvantage of this is approach are:

There could be multiple elements that would match provided selector and consequent misbehavior will be only observable at runtime. Let me use example following example: Say I have a UI component Note, that contains a Input. Clicking a Note activates focuses it's Input. If my outer components embeds multiple Notes clicking either one of them will end up focusing the same Notes Input unless of course you employ some name-spacing convention (and coordinated with embedded children) to avoid selector conflicts.

I would really prefer if Elm DOM provided a solution for identifying virtual DOM nodes (let's call them refs for now) that would allow uniquely identify a DOM element without selectors. In the other thread I have proposed something along these lines:

type alias Ref msg = HTML.Attribute msg

ref : model -> Ref msg

focus : Ref msg -> Task RefError a

Idea is to use model that UI element represents as an identifier for the view instead of requiring manual name-spacing solutions.

Currently I use slightly altered version of this approach where special ref field with unique HTML.Attribute msg is stored in the model & is used as in place of selector. Although that requires unique attribute value generation there for is side-effect so not great. But maybe something more like above proposed ref could be implemented in the core ?

@Gozala
Copy link

Gozala commented Jul 18, 2016

One more issue SelectorError is not enough, focus may silently fail (if element is not visible) here is an example:
https://jsbin.com/xeletez/edit?html,js,output

It would be great if task returned a different error when focusing failed rather than silently fail.

@Gozala
Copy link

Gozala commented Jul 18, 2016

I should also mention that running on the next render (assuming it's not on the same tick) may introduce race conditions. We've experienced them and switched to performing such task in the same tick. To be fare our issues were with selection management, but as far as I can tell focus management will be no different. So we had this race conditions where selection change task was scheduled to run on next tick, but in the meantime (especially if CPU is under stress) input value could change, there for wrong range will be selected. I imagine same could occur with focusing where you schedule a task to focus, in the meantime user focuses other input and then your task executes and user focused element get's blurred.

@gdotdesign
Copy link

Thanks for your input @rtfeldman.

Just to reiterate, the goal is to keep the API as small as possible. The central question I'm trying to answer is "can this be done without race conditions using just the function that returns a Task?" So far it still seems like the answer is "yes," right?

Well if you look on it that way probably the answer is yes, but consider this these:

  • the resulting code would be more complex than the one using the decoder (you have to manually keep track of many things that would be not needed)
  • it's fine using tasks until there is issue / problem where it's not enough and then it has to be implemented anyway

There is one more issue that comes to mind that would warrants a decoder and it's connected to drag and drop again:

  1. there is a container which have overflow: auto but doesn't show a scrollbar
  2. it has enough content that the dragged elmenet would show the scrollbar
  3. the dragger element is dragged over and the scrollbar shows modifying the bounds of every element inside
  4. there are two issues here:
    • the scrollbar width can vary between browsers (it can be measured though with JavaScript)
    • the change in overflow cannot be detected by JavaScript events, you could do it by hand by calculating from the containers bounds and the bounds of the children inside and the margins of those children, etc...

I have written down my input and I hope you consider adding these decoders for bounds and alike. It would help me a lot and I think it will be necessary sooner or later. I suggest you to create an implementation for a drag & drop system that is complex and have all the features that I mentioned, and judge by hand if the decoder should included.

Having said that I suggest we look into other aspects of this API and close this discussion, just let me know how you decided 😸

@debois
Copy link

debois commented Jul 19, 2016

I second @Gozala's reservations about CSS selectors. Note that for setters (eg, setScrolllTop), these reservations are completely resolved by an attribute-like API, as sketched in the out-of-bounds part of my comment above.

@Dremora
Copy link

Dremora commented Jul 19, 2016

I third @Gozala's Ref proposal.
Selectors are hard to write. One needs to be aware of the entire structure of the application in order to write a selector, which becomes a big burden as the app keeps growing. They can't be reliably used at all by 3rd party libraries, since any selector could be potentially matched by DOM created outside the library.
They easily leak into child views. I think we should only be able to focus an element we directly create, instead of relying on potentially complex and fragile DOM structure.
Selectors are prone to DOM structure changes.
The bottom line is, selectors are global (or in the best case scoped to an element and its descendants). They are the reason behind jQuery spaghetti code.
React has proven that Ref is a good and scalable abstraction.

@rtfeldman
Copy link
Author

Let's take the "ref" discussion to this thread

@rtfeldman
Copy link
Author

rtfeldman commented Jul 19, 2016

Added a note about focus silently failing to the gist. Also incorporated @fredcy's note about the typo of scrollLeft not showing up in function names, and removed setScrollTop and setScrollLeft based on @debois's point about how they make more sense as vdom implementations (which is already supported by calling Html.Attributes.property).

@rtfeldman
Copy link
Author

rtfeldman commented Jul 19, 2016

@gdotdesign thanks for the follow-up!

there are two issues here:

  • the scrollbar width can vary between browsers (it can be measured though with JavaScript)
  • the change in overflow cannot be detected by JavaScript events, you could do it by hand by calculating from the containers bounds and the bounds of the children inside and the margins of those children, etc...

This is still doable with the Task-based API, since the bounds can only change once you have dragged something into the container. At that point you can fire a Task to recompute the bounds, so you wouldn't have to do it on every mousemove in a race-condition-prone way - just when mousemove knows we need to expand the overflow: auto container to include the droppable element.

I suggest you to create an implementation for a drag & drop system that is complex and have all the features that I mentioned

As it happens, I have scratch-built a much more complex drag-and-drop system than this in JS using mousemove, mousedown, mouseup, and getClientBoundingRect, as well as the elementFromPoint trickery necessary to get touchmove to work similarly to how mousemove does on touch devices. 😺

In fact I hope to refactor that code to be written in pure Elm, so I expect to be writing a lot of code using whatever API ends up making it in...and I still value a minimal Elm API enough to prioritize it. 😉

Having said that I suggest we look into other aspects of this API and close this discussion, just let me know how you decided 😸

Thanks for the positive attitude! I'm going to stick with the Task-based one for now. We can always expand the API later if it proves insufficient. 😄

@rtfeldman
Copy link
Author

I should also mention that running on the next render (assuming it's not on the same tick) may introduce race conditions. We've experienced them and switched to performing such task in the same tick

Interesting...seems like both focus management and selection management may have more characteristics in common with the "some things need to run synchronously for security reasons" use cases than I thought. Maybe for them synchronous execution is the only way to avoid race conditions.

@mbylstra
Copy link

mbylstra commented Jul 20, 2016

I think div [ scrollTop 256 ] [ ... ] is a nice api (as suggested by @debois), but the semantics are unclear to me. Currently attributes specify the current state of the DOM: "The background color of this element is green", "This element is listening to mouseclick events". Updating these attributes is idempotent: it doesn't matter how often you set the background color to green - subsequent updates have no effect and there are no side effects. The VDOM will take care of unnecessarily updating the dom. However, scrollTop is fundamentally an imperative command with side effects.

Should Window.scrollTo() be fired every time the view function is called? What if Msgs are being fired every animation frame and the view function is called every animation frame? Will Window.scrollTo() be fired 60 frames per second while the scrollTop attribute is present? Or should it only be fired the first time the scrollTop attribute is introduced to the element? Or is the user of the API responsible for immediately removing the attribute so that it doesn't fire repeatedly? Does it make sense to serialise the state of the DOM with a scrollTop attribute present (server side rendering)?

Seeing as Msgs are not passed to the view function, how would you be able to add a scrollTop attribute in response to a mouse click? The only way I can see this being possible with the current view api is if you manually kept a state history in your model, and compared the current and previous states.

I am totally in agreeance that a querySelector based solution is not ideal, but I think a nicer solution would require a ground-up rethink of elm-html.

I've played with Reactive Native and I've found that the majority of the time you end up calling functions on components, rather than setting properties on them, which is far from the ideals of a declarative API. This stems from React Native being a wrapper on top of the extremely imperative iOS and Android UI APIs. For example, to open the Drawer (hamburger menu) in Android, you call the openDrawer() method on a component, rather than setting an isOpen = true property (see https://facebook.github.io/react-native/docs/drawerlayoutandroid.html#opendrawer). This is quite a bummer when you've been sold on the declarative way, but I think the reality is that you need to deal with imperative commands when integrating with third-party systems (such as the browser's DOM) that you can't feasibly re-implement from scratch.

I worry that without a nice framework for triggering commands on elements/components (without resorting to query selectors), a React Native equivalent in Elm is less feasible, and React Native is a huge drawcard for React.

Sorry for further derailing the conversation, but I think there are serious future implications to introducing a querySelector based API to core. I think it would be great if it existed as an unofficial third party library though (which unfortunately wouldn't be available on elm-package).

@rtfeldman
Copy link
Author

I think div [ scrollTop 256 ] [ ... ] is a nice api (as suggested by @debois), but the semantics are unclear to me. Currently attributes specify the current state of the DOM ...

I totally see your point, and I agree. Updated the gist to use set and get for scrollTop and scrollLeft.

@Gozala
Copy link

Gozala commented Jul 20, 2016

I also agree with @debois that declarative approach would be a better option. There for I would like to comment on the raised concerns here:

I think div [ scrollTop 256 ] [ ... ] is a nice api (as suggested by @debois), but the semantics are unclear to me. Currently attributes specify the current state of the DOM: "The background color of this element is green", "This element is listening to mouseclick events". Updating these attributes is idempotent: it doesn't matter how often you set the background color to green - subsequent updates have no effect and there are no side effects. The VDOM will take care of unnecessarily updating the dom. However, scrollTop is fundamentally an imperative command with side effects.

In fact value, selectionStart & selectionEnd of the input element are all very similar to scrollTop in regards that they can be mutated by user and if you are not listening to an appropriate DOM events and update your model accordingly your model & actual view state can get out of sync. Elm right now only deals with value, but same question(s) apply to it just as well: How ofter & when do you update value on the input field (I think all the other questions are really a subset of this one) ? Current answer is: On new render after model changes cause value of generated view to be updated.

Should Window.scrollTo() be fired every time the view function is called? What if Msgs are being fired every animation frame and the view function is called every animation frame? Will Window.scrollTo() be fired 60 frames per second while the scrollTop attribute is present? Or should it only be fired the first time the scrollTop attribute is introduced to the element? Or is the user of the API responsible for immediately removing the attribute so that it doesn't fire repeatedly? Does it make sense to serialise the state of the DOM with a scrollTop attribute present (server side rendering)?

I think answer to all of these should be same as changes to value on input are propagated to the view. If view updated it's scrollTop should be reflected onto corresponding DOM node. That has unfortunate consequence, which is if you want to control scroll position you need to listen to scroll events and update your model and view appropriately or else you get strange behavior again same as with value on input.

Which is why I think there is a room for both declarative APIs and task based APIs. I personally have being doing that for input selections. Sometimes you just want to select whatever is in input on some user interaction and leave all the other selection interactions to the platform. In such cases Task base API is great as you can get away without having to track and react to every value, cursor and selection changes. In other times you want to fully control the way selection works and in such cases declarative API is more convenient. BTW in react they also distinguish those two cases by referring to them as Controlled / Uncontrolled components (see https://facebook.github.io/react/docs/forms.html#controlled-components)

My understanding was that @rtfeldman want's to solve the "uncontrolled" use case first, which makes sense to me.

Seeing as Msgs are not passed to the view function, how would you be able to add a scrollTop attribute in response to a mouse click? The only way I can see this being possible with the current view api is if you manually kept a state history in your model, and compared the current and previous states.

If you like to control scroll position, position information would have to live in the model somewhere and you'd update it on messages that affect it. Scroll event would have to be one of them to stay in sync with actual view state.

@rtfeldman
Copy link
Author

Selectors are hard to write. One needs to be aware of the entire structure of the application in order to write a selector, which becomes a big burden as the app keeps growing.

This resonates with me, but so does this:

I am totally in agreeance that a querySelector based solution is not ideal, but I think a nicer solution would require a ground-up rethink of elm-html.

It'd be nice if the DOM exposed ways to interact with these things in declarative ways, but given that it does not, the querySelector API seems like the "least bad" option available.

@Dremora
Copy link

Dremora commented Jul 21, 2016

It'd be nice if the DOM exposed ways to interact with these things in declarative ways, but given that it does not, the querySelector API seems like the "least bad" option available.

It's not about DOM not exposing something, it's about potentially changing the way elm-html works (which, unlike the former, is in our control, now with the new version of virtual-dom even more than ever). querySelector is not the "least bad" option, it's the easiest to implement. But using it will throw us back to jQuery days (in some way worse, since in jQuery selectors were often scoped to an element).
I feel the problem of global CSS/selectors is underestimated in the Elm community, and I can't stress enough how much headache it has caused me in the past, regardless of how good the type system is or what kind of architecture is used for other aspects of the app. Until we solve it, we will not be able to build truly large and ambitious apps and reusable views+logic, where internal HTML structure of a view is truly opaque to its callers, and external HTML structure is opaque to a view. We should not need to know how to write a selector to reach a particular element, or think in terms of global DOM. We might not even be able to write a selector if a page is dynamic enough.

@rtfeldman
Copy link
Author

Thanks for all the comments! I don't feel like I have good solutions to the concerns raised about race conditions or the drawbacks of querySelector, so I summarized the discussion so far in order to give Evan a picture of where we are on this.

(He's finishing Guide stuff at the moment, and I know that's higher-priority, but based on past experiences I trust his judgment to resolve these points.)

Let me know on that thread if people think my summary missed anything important!

@debois
Copy link

debois commented Jul 21, 2016

Regarding scrollTop and @Gozala's neat comments above: In the current implementation of virtualdom, you cannot use, e.g., Html.Attributes.property "scrollTop" (Json.int 256) to set scrollTop. As it happens, properties are applied before children are populated, so on initial render, you're scrolling something with height 0. See here, default case applies.

@rtfeldman
Copy link
Author

rtfeldman commented Jul 22, 2016

@Gozala @debois I want to follow up a bit on this:

In fact value, selectionStart & selectionEnd of the input element are all very similar to scrollTop in regards that they can be mutated by user and if you are not listening to an appropriate DOM events and update your model accordingly your model & actual view state can get out of sync. Elm right now only deals with value, but same question(s) apply to it just as well: How ofter & when do you update value on the input field (I think all the other questions are really a subset of this one) ? Current answer is: On new render after model changes cause value of generated view to be updated.

Our production experience with value has been that it's irreparably broken. It has such nasty and unfixable race condition problems, our best practice has become to never use it and instead only to ever use defaultValue.

With value, users who type too fast (or are on older machines, or are using IE) report dropped characters and/or the cursor startlingly jumping to the end of the text field, leading them to suddenly type the wrong thing. Before we switched to defaultValue, our Webdriver-powered integration tests had to fill in form fields with one letter, because Webdriver "types" so fast that if we tried to have it type "Richard" into a form field, the form field would get filled in with some mangled version like "Ricrdha" because of value and its race condition problem, causing the test to fail.

I personally recommend that no one ever use value for input or textarea elements that users can type in, and I think having further APIs modeled after this (especially scrolling and text selection) would run into the same problems.

Granted, I can see how Task has its own problems with race conditions, but using a declarative Model -> Html approach to try and render anything where the Model is not the single source of truth for the state seems doomed to bugginess, and therefore a nonstarter. It seems like finding a way to have a Task run inside the same clock tick (which is a prerequisite for things like clipboard interactions where the browser's security model requires that anyway) is a more realistic path to explore.

What do you think?

@sylvainf911
Copy link

As I said in https://groups.google.com/d/msg/elm-dev/ThkWudq7SF0/77mbgGNoCwAJ the use of properties like scrollTop does not work. I did not experience problems with value but I did not use automated Webdriver like @rtfeldman.

One thing I had to create native lib for was adjusting the scrollTop after the content was changed in the view. When you scroll down and add some data to the model at the end of a list, there is no problem. But when you scroll up (prepend in the model's list) (think like Facebook messaging where you are at the end of the conversation first and autoload when scrolling up). You have to take the scrollHeight just before the DOM is updated, then update the DOM and then set scrollTop to a position equivalent of where it was before (el.scrollTop = el.scrollTop + (el.scrollHeight - oldHeight). Right now, I was not able to do it without a little glitch on the screen when repositioning. I don't know if this problem can be managed with the "in same tick" technique your are looking for.

Thank you.

@rtfeldman
Copy link
Author

I did not experience problems with value but I did not use automated Webdriver like @rtfeldman.

Being a race condition, reproducing it can be tricky depending on your device and such, but Kris did a good writeup of the problem including a demo of how to consistently reproduce it.

(Amusingly, later in that thread I suggested trying alternatives to defaultValue. I later talked about it with a bunch of people and concluded that defaultValue was the least bad alternative that actually solved the problem, similar to my conclusion here about querySelector. 😄)

@Gozala
Copy link

Gozala commented Aug 2, 2016

I have being thinking more about the problem of CSS selectors as poor way to reference Elements in the DOM and Declarative VS Task based approach for updating certain DOM properties. And I had a thought that there could be a way to solve both problems in a neat way. What if something along this lines was added to elm-html:

taskAttribute : (DOMElement -> Task Never msg) -> Html.Attribute msg

focus : DOMElement -> Task SelectorError msg
blur : DOMElement -> Task SelectorError msg
scrollLeft : Float -> DOMElement -> Task SelectorError Float

That way those task could be turned into attributes and elm-html can take care of running them with an appropriate DOM elements. Better yet it could actually recognize such tasks and run them in the same tick to avoid races we've talked about.

It not fully fleshed out idea, but I though I'd share it anyway to get other minds think about it.

@Gozala
Copy link

Gozala commented Aug 2, 2016

To be totally clear I don not suggest that Elm should expose raw DOMElement to Elm programs. It most definitely should not, but there could be a type like DOMElement (or Ref or whatever you want to call it) that is never given to an Elm program but can be used to connect different native APIs in this case connecting elm-html diff / patch process with with tasks like focus / blur that also likely will be implemented in native.

@Gozala
Copy link

Gozala commented Aug 2, 2016

I guess slightly adjusted API would more consistent with Task.perform and more convenient to use:

taskAttribute : (x -> msg) -> (a -> msg) -> (DOMElement -> Task x a) -> Html.Attribute msg

focus : DOMElement -> Task SelectorError a
blur : DOMElement -> Task SelectorError a
scrollLeft : Float -> DOMElement -> Task SelectorError Float

@Gozala
Copy link

Gozala commented Aug 2, 2016

Our production experience with value has been that it's irreparably broken. It has such nasty and unfixable race condition problems, our best practice has become to never use it and instead only to ever use defaultValue.

With value, users who type too fast (or are on older machines, or are using IE) report dropped characters and/or the cursor startlingly jumping to the end of the text field, leading them to suddenly type the wrong thing. Before we switched to defaultValue, our Webdriver-powered integration tests had to fill in form fields with one letter, because Webdriver "types" so fast that if we tried to have it type "Richard" into a form field, the form field would get filled in with some mangled version like "Ricrdha" because of value and its race condition problem, causing the test to fail.

I personally recommend that no one ever use value for input or textarea elements that users can type in, and I think having further APIs modeled after this (especially scrolling and text selection) would run into the same problems.

I commented in the referenced writeup but think it's worth brining this up here as well. In my experience (limited to the single browser engine) issue is not actually related to the race conditions and specifically blamed animation frames. But rather issue is related to imperfect modeling of the state and than unintentional mutation of DOM state. More specifically most of the code I've seen dealing with input fields does not model selection range and by consequence cursor position. It may not be apparent but when you do not model selection what you end up doing is actually modeling selection as being empty at the end of the input.value. So every time input.value is set selection state get's mutated and in a way that messes up input. In my experience tracking selection and modeling it in the state and then updating selection along with value resolved all the issues that seemed like race conditions. Better yet unlike react we kept animation frame based render loop which is not the issue. If your model state matches actual DOM state then later delayed DOM patches are NoOp and there for don't interfere with input value changes at all. I am happy to share more details in a different thread if there is an interest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment