Skip to content
This repository has been archived by the owner on Jun 4, 2024. It is now read-only.

Document discouraged elements, and fix ObjectEl data #178

Merged
merged 7 commits into from
Jan 30, 2021
Merged

Document discouraged elements, and fix ObjectEl data #178

merged 7 commits into from
Jan 30, 2021

Conversation

alexcjohnson
Copy link
Collaborator

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:

  • object (ObjectEl) - fixed so it can accept the data prop.
  • optgroup - AFAICT it's getting the label prop just fine, at least in Python. I haven't tried to render it though.
  • param - presumably this is working now that object is working.
  • track - I haven't tried, but I don't see why it wouldn't work.

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

  • blink
  • command
  • element
  • keygen

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

rpkyle
rpkyle approved these changes Jan 30, 2021
View reviewed changes
Copy link
Contributor

@rpkyle rpkyle left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thanks for adding this context (especially for htmlScript)! 💃

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

The first of these is considered obsolete, I believe, while the last two are deprecated. I'm not sure that Nextid is supported in any browser, htmlListing does work, while htmlSpacer only seems to work in very old versions of Firefox.

image
image
image

scripts/generate-components.js
* requires the oninput attribute of the enclosing <form> element, which
* is not accessible to Dash.`,
script: `
* CAUTION: <script> is included for completeness, but you cannot execute
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

@alexcjohnson
Copy link
Collaborator Author

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!

@alexcjohnson alexcjohnson merged commit 1148b6d into dev Jan 30, 2021
@alexcjohnson alexcjohnson deleted the 129-obsolete-docs branch January 30, 2021 22:59
@alexcjohnson alexcjohnson mentioned this pull request Jan 30, 2021
Merged
@rpkyle
Copy link
Contributor

rpkyle commented Jan 31, 2021

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!

@rpkyle rpkyle mentioned this pull request Apr 8, 2021
Merged
rpkyle added a commit that referenced this pull request Apr 8, 2021
@harryturr @rpkyle
@Marc-Andre-Rivet @dependabot @alexcjohnson @jbampton @ann-marie-ward
merge dev into master 1.1.3 (#185)
422c952 
* Use authenticated Docker pulls (#167)

* Update CODEOWNERS

* Bump dot-prop from 4.2.0 to 4.2.1

Bumps [dot-prop](https://github.com/sindresorhus/dot-prop) from 4.2.0 to 4.2.1.
- [Release notes](https://github.com/sindresorhus/dot-prop/releases)
- [Commits](sindresorhus/dot-prop@v4.2.0...v4.2.1)

Signed-off-by: dependabot[bot] <support@github.com>

* improve dash import test
see plotly/dash#1143

* changelog for import bug fix

* update extract-attributes script for latest MDN page structure
and provide an error message if we see the page structure change
in the future

* description for setProps - to reduce warnings

* use dash loosen-testing-reqs branch and fix linting

* Fix spelling

* back to dev branch of dash

* bump ci images versions

* Update config.yml

* update component gen for MDN update Jan 2021

* error message if element count changes from expectation

* add reference to MDN PR with the math & svg addition

* bump to v1.1.2 (#176)

* Remove context reference from CircleCI (#175)

* fix ObjectEl data prop

* add deprecation notes on some elements, and fix for a few more changes to MDN

* update tests, and add tests for custom docs and ObjectEl

* move percy to py37 and separate out finalize step

* more test cleanup

* one more try to combine percy snapshots

* changelog for #178

* Add `allow` prop to html.Iframe

* Add `referrerPolicy` prop and update tests

* Add `referrerPolicy` prop and update tests

* Updated changelog  Fixes #77

* Update CHANGELOG.md

* Bump elliptic from 6.5.3 to 6.5.4

Bumps [elliptic](https://github.com/indutny/elliptic) from 6.5.3 to 6.5.4.
- [Release notes](https://github.com/indutny/elliptic/releases)
- [Commits](indutny/elliptic@v6.5.3...v6.5.4)

Signed-off-by: dependabot[bot] <support@github.com>

* update toolchain

* Update CI images & py37 -> py39

* Bump y18n from 4.0.0 to 4.0.1

Bumps [y18n](https://github.com/yargs/y18n) from 4.0.0 to 4.0.1.
- [Release notes](https://github.com/yargs/y18n/releases)
- [Changelog](https://github.com/yargs/y18n/blob/master/CHANGELOG.md)
- [Commits](https://github.com/yargs/y18n/commits)

Signed-off-by: dependabot[bot] <support@github.com>

* remove css

* version bump 1.1.3 (#184)

* version bump to 1.1.3

* update changelog

* artifacts

Co-authored-by: Ryan Patrick Kyle <rpkyle@users.noreply.github.com>
Co-authored-by: Marc-André Rivet <Marc-Andre-Rivet@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: alexcjohnson <alex@plot.ly>
Co-authored-by: Alex Johnson <johnson.alex.c@gmail.com>
Co-authored-by: John Bampton <jbampton@gmail.com>
Co-authored-by: Marc-André Rivet <marc-andre.rivet@plotly.com>
Co-authored-by: Ann Marie Ward <amward@fastmail.us>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rpkyle rpkyle rpkyle approved these changes

@Marc-Andre-Rivet Marc-Andre-Rivet Awaiting requested review from Marc-Andre-Rivet

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@alexcjohnson @rpkyle