Skip to content

Conversation

@jonels-msft
Copy link
Contributor

No description provided.

@jonels-msft jonels-msft changed the title Fix contrast with Citus docs color theme Improve docs color contrast for accessibility Apr 21, 2021
@jonels-msft
Copy link
Contributor Author

Don't merge yet, still a work in progress.

@DimCitus DimCitus self-requested a review April 22, 2021 10:24
@DimCitus DimCitus added docs The documentation needs more work Size:M Effort Estimate: Medium labels Apr 22, 2021
@DimCitus DimCitus added this to the Sprint 2021 W16 W17 milestone Apr 22, 2021
@jonels-msft
Copy link
Contributor Author

@DimCitus this is ready for review now.

What I'm most uncertain about is whether Read The Docs will honor the requirements.txt file I added in this PR. I'm using that file to attempt to pick the latest Sphinx and sphinx_rtd_theme packages. We'll need to bump their versions in the future when other accessibility changes get merged upstream.

@JelteF
Copy link
Contributor

JelteF commented Apr 23, 2021

What I'm most uncertain about is whether Read The Docs will honor the requirements.txt file I added in this PR.

Based on the "read the docs" docs it is possible to do this, but we need to add a .readthedocs.yaml file to the repo.

@jonels-msft
Copy link
Contributor Author

Thanks @JelteF !

I tested the change on a branch of citus_docs and confirmed in the build log that RTD installed the right versions. Also checked the result, and the docs rendered fine. So the change should be good for pg_auto_failover docs.

Copy link
Collaborator

@DimCitus DimCitus left a comment

Choose a reason for hiding this comment

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

Thanks for the accessibility work @jonels-msft ! I did a quick review of the docs created locally with your patch and it looks all good. About the requirements thing on the read the docs website, I have added your branch in their build system and it is now published over at https://pg-auto-failover.readthedocs.io/en/jonels-contrast/ for your review.

@jonels-msft
Copy link
Contributor Author

@dimitri hmm, something went wrong with the PR merge. For instance I don't see add_css_file() in docs/conf.py

I'll open a new PR with my changes rebased against master.

@DimCitus
Copy link
Collaborator

@dimitri hmm, something went wrong with the PR merge. For instance I don't see add_css_file() in docs/conf.py

I'll open a new PR with my changes rebased against master.

I though I could click “Update Branch” in the GitHub UI and then merge the PR in my TZ, and we've been busy with other things. I'll wait until you ack the current PR before merging then. Feel free to override the GH changes by pushing what you have on top of the current state of the PR, too.

@jonels-msft
Copy link
Contributor Author

I rebased and force pushed. This PR should be good to merge when you're ready.

@DimCitus
Copy link
Collaborator

You need to do a make indent pass and then push again, the auto-formatting check fails on Travis, see https://travis-ci.com/github/citusdata/pg_auto_failover/jobs/502049017

$ if [ -n "$LINTING" ]; then black --check .; fi
would reformat docs/conf.py
Oh no! 💥 💔 💥
1 file would be reformatted, 22 files would be left unchanged.
The command "if [ -n "$LINTING" ]; then black --check .; fi" exited with 1.

@DimCitus DimCitus merged commit f36fb31 into master Apr 29, 2021
@DimCitus DimCitus deleted the jonels-contrast branch April 29, 2021 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs The documentation needs more work Size:M Effort Estimate: Medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants