Skip to content
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

Release cycle chart: show suggestion if JavaScript disabled #997

Merged
merged 1 commit into from
Dec 6, 2022

Conversation

hugovk
Copy link
Member

@hugovk hugovk commented Dec 5, 2022

Follow on from #988.

Visit https://devguide.python.org/versions/.


Before: with JavaScript enabled

image


Before: with JavaScript disabled

image


After: with JavaScript disabled

Instead, hide chart code and advise the user to enable JS if they wish to see the chart:

image

Uses the advice at https://florianbrinkmann.com/en/css-rules-when-javascript-is-disabled-4536/


Demo

https://cpython-devguide--997.org.readthedocs.build/versions/

Copy link
Member

@ezio-melotti ezio-melotti left a comment

Choose a reason for hiding this comment

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

This is an improvement compared to what we have now, but have you considered creating a static image from mermaid at build time (if possible)? This will make it easier to share/embed too.

@CAM-Gerlach
Copy link
Member

have you considered creating a static image from mermaid at build time (if possible)? This will make it easier to share/embed too.

Seems like that could have a number of issues:

  • Would need to be regenerated regularly to keep the current date line (and status/colors) accurate and up to date, which is adds complexity and noise
  • Less efficient to store in the repo forever every time it is updated, exacerbated by the above
  • Would increase bandwidth cost and requests needed to load the page (though probably reduce local CPU load)
  • Static embedded rather than inline SVG would not be directly style-able via CSS
  • Seems likely to make it more complicated for contributors/RMs to regenerate the image locally, if it requires NodeJS and JavaScript dependencies to be installed

I'm not sure the time, effort and complexity to set up and add to the CI and local workflows is worth it for the tiny proportion of users with JS disabled, but maybe the share/embed use case is more compelling?

Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

LGTM, FWIW—too bad we have to rely on raw, but I guess it isn't so bad for the devguide.

@hugovk
Copy link
Member Author

hugovk commented Dec 6, 2022

I'll have a look into pre-generating an image, it can do SVG or PNG:

https://sphinxcontrib-mermaid-demo.readthedocs.io/en/latest/#config-values

But as noted it requires installing and running Node.js.

We could instead use JavaScript to update the date line for an SVG, but then again we could instead embed the Google chart like https://python-release-cycle.glitch.me/

Anyway, let's merge this, thanks for the reviews and suggestions!

@hugovk hugovk merged commit c4d2e67 into python:main Dec 6, 2022
@hugovk hugovk deleted the mermaid-noscript branch December 6, 2022 09:20
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

Successfully merging this pull request may close these issues.

4 participants