Skip to content

"dash length list in px" option not specified in schema #2903

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

Closed
jonmmease opened this issue Aug 14, 2018 · 6 comments
Closed

"dash length list in px" option not specified in schema #2903

jonmmease opened this issue Aug 14, 2018 · 6 comments

Comments

@jonmmease
Copy link
Contributor

See plotly/plotly.py#1107.

Plotly.py version 3 is not allowing a scatter.line.dash property to be set to a list of lengths in pixels (e.g. '5px,10px,2px,2px'). The issue is that the schema lists the property as an enumeration, and doesn't indicate this pixel list form:

                    "dash": {
                        "valType": "string",
                        "values": [
                            "solid",
                            "dot",
                            "dash",
                            "longdash",
                            "dashdot",
                            "longdashdot"
                        ],
                        "dflt": "solid",
                        "role": "style",
                        "editType": "style",
                        "description": "Sets the dash style of lines. Set to a dash type string (*solid*, *dot*, *dash*, *longdash*, *dashdot*, or *longdashdot*) or a dash length list in px (eg *5px,10px,2px,2px*)."
                    },

How would you recommend proceeding in plotly.py? Should dashes be a special case (which is fine on my end), or is there something that we should do in the schema?

Thanks!

@jonmmease
Copy link
Contributor Author

Thinking more about it, could we add a regular expression to the values list that would handle this case?

@alexcjohnson
Copy link
Collaborator

Right, we have valType: 'string' so internally we don't actually use the values at all. The code has this comment:

    // string type usually doesn't take values... this one should really be
    // a special type or at least a special coercion function, from the GUI
    // you only get these values but elsewhere the user can supply a list of
    // dash lengths in px, and it will be honored

For now I'd make this a special case, but @etpinard what about folding this into enumerated and adding a field like

regExpValues: [/^(\d+(\.\d+)?px,)*\d+(\.\d+)?px$/]

ie in principle a list of RegExp (but we only need one here) that during PlotSchema.get() we convert into a list of strings

regExpValues: ['^(\d+(\.\d+)?px,)*\d+(\.\d+)?px$']

(which is slightly more complicated than just .toString() because that leaves the / bookends that other languages won't want... but that's easy to fix.)

Oddly, although we've always used px as part of the string, it's really not supposed to be there and seems to just be ignored by all the browsers we support. So I guess we should continue to support it but make it optional and adapt the docs to discourage it. And you can use commas or whitespace between items. And then there's also an option to specify percentages, that I can see occasionally being convenient...
https://www.w3.org/TR/SVG/painting.html#StrokeDashing

So the ideal regexp might be: /^(\d+(\.\d+)?(px|%)?(,|\s))*\d+(\.\d+)?(px|%)?$/

@etpinard
Copy link
Contributor

Yep, regExpValues sounds like a good way forward.

About toString, it should be too hard to modify

// as JSON.stringify(/test/) // => {}
if(attr[k] instanceof RegExp) {
attr[k] = attr[k].toString();
}

to output what we want in the schema json.

@jonmmease
Copy link
Contributor Author

Thanks @alexcjohnson and @etpinard,

I'll make it a special case and use @alexcjohnson 's regular expression as a starting point for our own validation. Even if we do add the regular expression to the schema, a special case is probably more user-friendly than telling a user that their string must match this regular expression 🙂

Here's the full list of traces that have a line.dash property.

screen shot 2018-08-15 at 6 21 37 am

Based on the descriptions, it looks like all of them except scatterpolargl, scattergl, and scatter3d support the custom length dashes. Does that sound right?

@etpinard
Copy link
Contributor

all of them except scatterpolargl, scattergl, and scatter3d support the custom length dashes. Does that sound right?

That's correct 👍

@gvwilson
Copy link
Contributor

Hi - this issue has been sitting for a while, so as part of our effort to tidy up our public repositories I'm going to close it. If it's still a concern, we'd be grateful if you could open a new issue (with a short reproducible example if appropriate) so that we can add it to our stack. Cheers - @gvwilson

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

No branches or pull requests

4 participants