Text() attribute styling/spacing

[mrcook]

There are issues with how Text() is generated, regarding both spacing and the use of style...at least when viewing in Chrome browser.

Examples:

canvas.Text(300, 150, "Some Text", "font-size:48px;", `fill="#222222"`)
<text x="300" y="150" style="font-size:48px;"fill="#222222" >Some Text</text>

canvas.Text(300, 150, "Some Text", "font-size:48px;", "fill:white")
<text x="300" y="150" style="font-size:48px;" style="fill:white">Some Text</text>

In the first example, you can see that the fill attribute has no spacing before it which in Chrome, produces an attributes construct error message. Also, there is an unnecessary space before the closing >.

In the second example (after manually inserting that space before style) I get an Attribute style redefined message.

This is fixed very easily in endstyle(), which would also benefit from a little refactoring.

[ajstarks]

do you have a suggested fix? What Go version did you build the library with? What version of Chrome are you seeing the error?

[mrcook]

As a quick fix I just added a space before each attribute, but looking at the code, you'd likely want to do a somewhat more elegant refactor.

I'm on the latest versions of everything: Go 1.9; Chrome 61.0.3163.91 (64-bit) -- but also on Chrome 60.0.3112.113 I get the same issues.

My system is Mac OS X 10.11.6.

[mrcook]

As additional info: FireFox 55.0.3 has the same behaviour, as does Safari 11.0 (11604.1.38.1.7)

[ajstarks]

Note that the variadic arguments were designed to be the name="value" type. The use case is to use the variadic type if the style= type is not sufficient. (this is what you get if you simply use a single argument.

Try this:

package main

import (
    "os"

    "github.com/ajstarks/svgo"
)

func main() {
    canvas := svg.New(os.Stdout)
    width, height := 600, 600
    canvas.Start(width, height)
    canvas.Rect(0, 0, width, height, "fill:red")
    canvas.Text(200, 150, "Some Text", `font-size="48px"`, `fill="#ababab"`)
    canvas.Text(200, 100, "More Text", `font-size="48px"`, `fill="white"`)
    canvas.End()
}

[mrcook]

Sure, I understand the issue doesn't show up if you supply just regular attributes, but this is not enforced in any way. I guess the main problem is that you automatically create a style attribute, so if something like font-size:48px is given, this will produce: style="font-size:48px".

Perhaps a better approach would be not to do that auto convert, and if a user wishes to use the style attribute they would have to specify it directly like so:

canvas.Text(200, 150, "Some Text", `style="font-size:48px;fill:blue;"`)

[mrcook]

...although maybe a better approach would be to accept a map instead of just a plain old string:

map[string]string{
  "font-size": "48px",
  "fill": "#ababab",
}

This would make it very obvious what was expected.

Just some thoughts.

[ajstarks]

Not doing the auto convert would break a lot of code. In fact the auto style= was meant as a shorthand because most of the time it was sufficient, and the variadic case was for when you needed complete control.

There may be better ways, but I'm afraid I'm stuck with it now. Perhaps as you suggest better error handing of the variadic case would help.