plotly / dash-html-components

OBSOLETE - now part of https://github.com/plotly/dash
https://dash.plotly.com
Other
153 stars 49 forks source link

Document discouraged elements, and fix ObjectEl data #178

Closed alexcjohnson closed 3 years ago

alexcjohnson commented 3 years ago

Fixes #161 - ObjectEl now supports the data prop. Fixes #129 - I did not remove any elements, but prominently documented their behavior in the react class docstring, which makes its way into all the built components (and from there will be shown in dash-docs) like:

MDN changed their elements reference page again - seems like post-rewrite it's become less stable as more people are contributing to it. They added one element (portal) and removed two (command and element) - for now I just set these aside, kept the set of components we generate the same.

    """A Basefont component.
Basefont is a wrapper for the <basefont> HTML5 element.

OBSOLETE: <basefont> is included for completeness, but should be avoided
as it is only supported by Internet Explorer.

For detailed attribute info see:
...

Here's the note I added to script, which @mbauman noted is particularly important as folks are likely to try and use this to execute arbitrary JS:

    """A Script component.
Script is a wrapper for the <script> HTML5 element.

CAUTION: <script> is included for completeness, but you cannot execute
JavaScript code by providing it to a <script> element. Use a clientside
callback for this purpose instead.

For detailed attribute info see:
...

I added notes to all elements listed in #129 except:

I also added notes to these obsolete elements not listed in #129:

Along the way I updated the test suite to use dash.testing. I had trouble getting percy to properly collect all snapshots into one run. Doesn't seem like we should need the --nopercyfinalize, PERCY_PARALLEL_TOTAL: -1 and separate percy-finalize job since there's only one job submitting snapshots to percy and it has no parallelism, but without all of that it kept breaking each test case into a separate build. Perhaps something to look into in dash.testing

alexcjohnson commented 3 years ago

As an aside, is it worthwhile to add notes for the following components also?

htmlNextid htmlSpacer htmlListing

Yes, I gave all of these OBSOLETE messages "should be avoided as it is not supported by any modern browsers." Those were included in the set "added notes to all elements listed in #129 except ..."

I didn't do an exhaustive search through all the elements, just took the ones that seemed obvious or likely to be misused. If anyone wants to extend these notes to other elements in the future, PRs absolutely welcome!

rpkyle commented 3 years ago

As an aside, is it worthwhile to add notes for the following components also? htmlNextid htmlSpacer htmlListing

Yes, I gave all of these OBSOLETE messages "should be avoided as it is not supported by any modern browsers." Those were included in the set "added notes to all elements listed in #129 except ..."

I didn't do an exhaustive search through all the elements, just took the ones that seemed obvious or likely to be misused. If anyone wants to extend these notes to other elements in the future, PRs absolutely welcome!

Ah, OK, I see that now. Thanks again!