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

Use read the docs sphinx theme instead of bootstrap theme #2825

Conversation

johannaengland
Copy link
Contributor

@johannaengland johannaengland commented Feb 22, 2024

Closes #2805.

@johannaengland johannaengland added the documentation Related to documentation of NAV label Feb 22, 2024
@johannaengland johannaengland self-assigned this Feb 22, 2024
@johannaengland johannaengland force-pushed the docs/remove-sphinx-bootstrap-theme branch from b6c6cd0 to 267663a Compare February 22, 2024 14:24
Copy link

codecov bot commented Feb 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 57.10%. Comparing base (de2a90b) to head (34979a3).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2825   +/-   ##
=======================================
  Coverage   57.10%   57.10%           
=======================================
  Files         567      567           
  Lines       41278    41278           
=======================================
  Hits        23571    23571           
  Misses      17707    17707           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

github-actions bot commented Feb 22, 2024

Test results

     12 files       12 suites   12m 10s ⏱️
3 303 tests 3 303 ✔️ 0 💤 0
9 384 runs  9 384 ✔️ 0 💤 0

Results for commit 34979a3.

♻️ This comment has been updated with latest results.

@johannaengland johannaengland force-pushed the docs/remove-sphinx-bootstrap-theme branch 6 times, most recently from c4856b1 to a9d9236 Compare February 22, 2024 14:57
@johannaengland johannaengland added the discussion Requires developer feedback/discussion before implementation label Feb 22, 2024
@hmpf
Copy link
Contributor

hmpf commented Feb 26, 2024

The easiest way to check the result of this PR is to visit https://nav--2825.org.readthedocs.build/en/2825/ btw, readthedocs builds the docs for us.

@hmpf
Copy link
Contributor

hmpf commented Feb 26, 2024

I think it looks perfectly fine as is. @lunkwill42?

@lunkwill42
Copy link
Member

I think it looks perfectly fine as is. @lunkwill42?

Looks perfectly cromulent to me. And this theme is more likely to be maintained as well.

If we can somehow get the logo back on small screen views, that would also be great :)

Copy link
Contributor

@stveit stveit left a comment

Choose a reason for hiding this comment

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

I like it. Feels easier to navigate since i'm already used to the readthedocs format from other documentations. When using the old NAV docs i usually have to search to find anything, but this I feel can actually be navigated

Copy link
Contributor

@podliashanyk podliashanyk left a comment

Choose a reason for hiding this comment

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

Looks good to me.
Some things could be improved, but nothing critical:

  1. Agree with @lunkwill42's comment about the logo.
  2. This theme introduces new contrast errors, f.e. in the footer text.

@johannaengland johannaengland marked this pull request as ready for review February 28, 2024 07:35
@johannaengland johannaengland force-pushed the docs/remove-sphinx-bootstrap-theme branch from a9d9236 to 34979a3 Compare February 28, 2024 07:45
Copy link

sonarcloud bot commented Feb 28, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@johannaengland johannaengland merged commit 2f19a39 into Uninett:master Feb 28, 2024
12 checks passed
@johannaengland johannaengland deleted the docs/remove-sphinx-bootstrap-theme branch February 28, 2024 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Requires developer feedback/discussion before implementation documentation Related to documentation of NAV
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check the sphinx theme
5 participants