Docs: a start on an 'improve this page' feature#136246
Docs: a start on an 'improve this page' feature#136246nedbat wants to merge 16 commits intopython:mainfrom
Conversation
nedbat
force-pushed
the
nedbat/improve-docs-page
branch
from
b8f50bc to
673e347
Compare
StanFromIreland
left a comment
StanFromIreland
left a comment
There was a problem hiding this comment.
This looks great. I have some little thoughts.
Also, this should probably get an issue and blurb?
Doc/improvepage.rst
Outdated
| `Docs: problem with "PAGETITLE" <https://github.com/python/cpython/issues/new?title=Docs%3A+problem+with+%22PAGETITLE%22&labels=docs&body=The+page+at+PAGEURL+has+a+problem%3A>`_. | ||
|
|
||
| - You can `edit the page on GitHub <https://github.com/python/cpython/blob/main/Doc/PAGESOURCE?plain=1>`_ | ||
| and open a pull request, though you will need to have signed a contributor agreement before it can be merged. |
There was a problem hiding this comment.
Just a little note, I think mentioning the contribution agreement here may scare away people, as it does not explain much about it, and does not pop up when they edit the page.
There was a problem hiding this comment.
Yes, I was torn about putting it here, we can drop it if needed.
There was a problem hiding this comment.
I changed it to "open a pull request and begin the contribution process." to at least hint at the idea that there will be steps needed.
|
I like the "This link will start a pre-populated topic:" link, but I wonder whether "less is more" applies here. In particular, I mean if the "Improve this page" link directly goes to the pre-populated Discourse form, would that more directly get users to where they can make a suggestion? If so, would it be better for "Improve this page" to link straight to Discourse? |
I'm not sure Discourse is the right place for everyone. Some people will have edits, some will have questions, some will want to discuss ideas for improvement. I appreciate the desire to make the path as well-greased as possible. If we do decide to link straight to Discourse, we'll need to change the link from "Improve this page", because Discourse is not the place to do that directly. |
| <script> | ||
| document.addEventListener('DOMContentLoaded', () => { | ||
| const params = new URLSearchParams(window.location.search); | ||
| document.body.innerHTML = document.body.innerHTML | ||
| .replace(/PAGETITLE/g, params.get('pagetitle')) | ||
| .replace(/PAGEURL/g, params.get('pageurl')) | ||
| .replace(/PAGESOURCE/g, params.get('pagesource')); | ||
| }); | ||
| </script> |
There was a problem hiding this comment.
Can we avoid inline JS? Ideally both here and the <script> in the template would be moved to .js files.
There was a problem hiding this comment.
There are already two instances of inline script, and tbh, for small one-off chunks like this it might be more understandable to have them on the page where they are used. But in any case, would it be OK to land this feature first and then decide on centralization?
AA-Turner
left a comment
AA-Turner
left a comment
There was a problem hiding this comment.
I'd like to see if we can make this work as far as reasonably practical without JS -- we currently don't require users to have JS enabled to use the documentation.
On Hugo's point on translations, we could add another .. only:: block for non-English containing a brief description of changes to the translation vs source content.
A
For reference, the current practice for bugs.po, per PEP 545, is to stick it in somewhere at the end of an existing message. I’m not a Sphinx expert but I don’t see how it would work, from my experience it can only exclude builders. If you could please share some Sphinx magic I’d love to open a PR for bugs. |
I'm happy to try another approach, but I'm not sure what we can do. The goal is for the Improve page to know what page the user was reading. Do you have another way? |
- use the actual page URL - tighten the wording
Maybe I don't understand the translation issue. What is the |
See #137449 for |
|
BTW, #81792 also discusses ideas for these pages. Anyone have changes needed to this PR before we merge and iterate? |
Yes, see above. I'll have a quick go at a less-JS version, apologies for not seeing your reply. |
|
I lost track of the PR. What are people's opinions now? |
|
These are pretty short chunks of JavaScript so I'm fine with them. We can always replace with non-JS later. But after clicking "Improve this page" when JS is disabled we get a load of placeholders: 
So we'll need some fallback when JS is disabled or unavailable. |
Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
|
I've dealt with the "No JS" problem by having a separate -nojs version of the page which is linked in the sidebar. The existing JavaScript snippet changes the URL to the js-enabled version of the page. |
Doc/improve-page.rst
Outdated
| <script> | ||
| document.addEventListener('DOMContentLoaded', () => { | ||
| const params = new URLSearchParams(window.location.search); | ||
| document.body.innerHTML = document.body.innerHTML |
There was a problem hiding this comment.
This is vulnerable to a DOM based XSS, no? We should escape the params.
There was a problem hiding this comment.
Excellent point, and I should have known better. I will fix it.
Doc/improve-page.rst
Outdated
|
|
||
| - You can open an issue on the Python GitHub issue tracker. This link will | ||
| create a new pre-populated issue: | ||
| `Docs: problem with page "PAGETITLE" <https://github.com/python/cpython/issues/new?title=Docs%3A+problem+with+page+%22PAGETITLE%22&labels=docs&body=The+page+at+PAGEURL+has+a+problem%3A>`_. |
There was a problem hiding this comment.
Similarly to above,
| `Docs: problem with page "PAGETITLE" <https://github.com/python/cpython/issues/new?title=Docs%3A+problem+with+page+%22PAGETITLE%22&labels=docs&body=The+page+at+PAGEURL+has+a+problem%3A>`_. | |
| `Docs: problem with page "PAGETITLE" <https://github.com/python/cpython/issues/new?title=Docs%3A+problem+with+page+%22PAGETITLE%22&template=documentation.yml&body=The+page+at+PAGEURL+has+a+problem%3A>`_. |
I expect this will also require updating body.
There was a problem hiding this comment.
The URL works to create the title and body, but does not apply the label, as tested here: #144780. I think this is fine.
Doc/improve-page-nojs.rst
Outdated
|
|
||
| - You can open an issue on the Python GitHub issue tracker. This link will | ||
| create a new issue with the "docs" label: | ||
| `New docs issue <https://github.com/python/cpython/issues/new?labels=docs>`_. |
There was a problem hiding this comment.
I think this (specifically labels=docs) link won't work for anyone who doesn’t have triage permissions in CPython (otherwise, spammers could create "triaged" issues), I suggest linking to the template instead, i.e.: https://github.com/python/cpython/issues/new?template=documentation.yml
There was a problem hiding this comment.
Here the template is a better choice, I'll change it.
|
OK, I've used the documentation.yml template from both pages. It lets me set a title but not a body, but will apply the docs label no matter who uses it. |
This need not be the case :-) You can add an |
6 participants
At the Docs WG meeting, we talked about smoothing the path for people to suggest improvements to the docs. This is a start. It uses more JavaScript than we are used to.
I haven't done anything to trim down the "Report a bug" page. Ideas welcome.
📚 Documentation preview 📚: https://cpython-previews--136246.org.readthedocs.build/