elm / virtual-dom

The foundation of HTML and SVG in Elm.
https://package.elm-lang.org/packages/elm/virtual-dom/latest
BSD 3-Clause "New" or "Revised" License
209 stars 80 forks source link

The "namespace" property on VirtualDom.node is ignored. #133

Closed rupertlssmith closed 6 years ago

rupertlssmith commented 6 years ago

Elm 0.91, Debian Stretch (or Ellie)

SSCCE:

module Main exposing (main)

import Browser
import Html exposing (Html,  div, text)
import Json.Encode as Json
import TypedSvg exposing (..)
import TypedSvg.Attributes exposing (..)
import TypedSvg.Types exposing (px)
import VirtualDom exposing (Attribute, Node)

type alias Model =
    Int

initialModel : Model
initialModel =
    0

type Msg
    = None

update : Msg -> Model -> Model
update msg model =
    case msg of
        None -> model

view : Model -> Html Msg
view model =
    div []
        [ div [] [ text <| "SVG Round Rectangle" ]
        , roundRect
        , roundRect2
        ]

roundRect : Html.Html msg
roundRect =
    (node "svg")
        [ width (px 120), height (px 120), viewBox 0 0 120 120 ]
        [ rect [ x (px 10), y (px 10), width (px 100), height (px 100), rx (px 15), ry (px 15) ] [] ]

roundRect2 : Html.Html msg
roundRect2 =
    (nodeNS "svg")
        [ width (px 120), height (px 120), viewBox 0 0 120 120 ]
        [ (nodeNS "rect") [ x (px 10), y (px 10), width (px 100), height (px 100), rx (px 15), ry (px 15) ] [] ]

main : Program () Model Msg
main =
    Browser.sandbox
        { init = initialModel
        , view = view
        , update = update
        }

svgNamespace : Attribute msg
svgNamespace =
    VirtualDom.property "namespace" (Json.string "http://www.w3.org/2000/svg")

node : String -> List (Attribute msg) -> List (Node msg) -> Node msg
node name =
    \attributes children ->
        VirtualDom.node name (svgNamespace :: attributes) children

nodeNS : String -> List (Attribute msg) -> List (Node msg) -> Node msg
nodeNS =
  VirtualDom.nodeNS "http://www.w3.org/2000/svg"  

When run, only one rounded rectangle is shown. This is because in the roundRect the DOM nodes are built with VirtualDom.node. Even though the "namespace" property is set on them, they are not namespaced and their attributes are also lower-cased (which is the correct behaviour for the browser for non-namespaced nodes).

The namespace can be set correctly using VirtualDom.nodeNS - which is what its for after all!

Should the node function accept namespaces set as a property, it did in 0.18?

If this is not an oversight or genuine issue, I propose instead to submit a PR just adding a little note to the virtual-dom documentation to explain this.

evancz commented 6 years ago

Is it possible to make the example smaller? It seems like the core of it is about property "namespace", and the claim seems to be that it worked one way in 0.18 but another way in 0.19. Is it possible to take a look at the code and see why it may have worked in the past? I am surprised that it did.

evancz commented 6 years ago

I ran across an issue that led to the fix here: https://github.com/elm/svg/commit/62b46c83280fa0297a814747929c4fa96a86cfef

That demonstrates pretty clearly that things used to work with property namespace. I guess some of the work I did to make virtual-dom smaller included switching to the nodeNS way of setting this. (I recall it needed a special case when it was a property.) This probably happened quite early in the 0.19 process, so I am not 100% on the details. Enough that I thought it was always with nodeNS!