support target and popup attributes in pseudo-html text links #1726
Conversation
| * note that at least in Chrome, you need to give at least one property | ||
| * in this string or the page will open in a new tab anyway. We follow this | ||
| * convention and will not make a popup if this string is empty. | ||
| * per the spec, cannot contain whitespace. |
There was a problem hiding this comment.
seems to be a point of browser inconsistency whether it still works with whitespace... alternatively I guess we could accept whitespace in the input but strip it from the output, but this was easier.
| @@ -297,14 +322,14 @@ exports.plainText = function(_str) { | |||
| }; | |||
|
|
|||
| function replaceFromMapObject(_str, list) { | |||
| var out = _str || ''; | |||
| if(!_str) return ''; | |||
There was a problem hiding this comment.
馃悗 no need to loop over all these replacements on an empty string
Excellent point. Our pseudo-html applies to really anywhere we display text: annotations, plot and axis titles, trace names. Also tick labels (categories, ticktext, tickprefix, ticksuffix...) and trace text and hovertext though those are generally unclickable so this particular change doesn't matter there. Seems like this format is too complex to include outright in all of these attribute, so we need some dedicated text formatting page that all of these attributes can somehow reference? |
馃憤 |
|
Maybe open up an issue on https://github.com/plotly/documentation/issues/new so we don't forget about it. |
|
馃悕 looks good to me. |
Adds two new attributes for more flexibility to determine where links in our pseudo-html text will open:
target, which is preferred overxlink:showin modern browsers and has more functionality - but I still includexlink:showas a fallback:'replace'if target is'_self','_parent', or'_top'and'new'for'_blank'or a named frame. Somexlink:showimplementations seem to accept more values to partially bridge the gap withtargetbut some don't, and I believe most that do will look attargetanyway.popup, which doesn't exist in html but we need a way to insert the "window features" string without accepting arbitrary javascript, so inventing a new attribute seemed like the easiest way. Note thattargetis sent as the second arg (name) towindow.open, which lets you send subsequent requests to the same popup (or, uselessly, to not make a popup at all if you use'_self'etc)So usage would look like:
'<a href="https://plot.ly" target="fred" popup="width=500,height=400,menubar=no">link</a>'@etpinard look reasonable?
@scjody quick 馃悕 review? I poked at it for a while and couldn't find any problems but would love a second set of eyes. Note that both attribute values get
encodeForHTMLimmediately on extraction.