Skip to content

Added favicon to user documentation #18007

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

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

ManolisPapa
Copy link

@ManolisPapa ManolisPapa commented Apr 24, 2025

Link to issue number:

Fixes #17971

Summary of the issue:

When loading NVDA HTML documentation in the browser, there's no page icon (favicon.ico).

Description of user facing changes:

The NVDA logo is visible when opening any .html file in the user_docs

Description of development approach:

I added a simple rel="shortcut icon" similar to nvaccess' website using the NVDA logo which I found here. I also changed the logic in sconstruct.py to make sure that the favicon is pulled locally in every subfolder and cleaned when running scons -c

Testing strategy:

Run .\scons.bat user_docs to generate the .html files and the favicon.ico
Run .\scons.bat -c user_docs to clear out the generated files

Known issues with pull request:

Code Review Checklist:

  • Documentation:
    • Change log entry
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • API is compatible with existing add-ons.
  • Security precautions taken.

@coderabbitai summary

@ManolisPapa ManolisPapa requested a review from a team as a code owner April 24, 2025 11:13
@@ -331,6 +331,7 @@ for po in env.Glob(sourceDir.path + "/locale/*/lc_messages/*.po"):

Copy link
Member

Choose a reason for hiding this comment

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

please also add a reference in

"*/user_docs/styles.css",

@seanbudd
Copy link
Member

can this also please be rebased to beta?

@seanbudd
Copy link
Member

Rather than committing a new file, can you just use source\images\nvda.ico?

@seanbudd seanbudd added this to the 2025.1 milestone Apr 28, 2025
@seanbudd seanbudd marked this pull request as draft April 28, 2025 02:01
@@ -331,6 +331,7 @@ for po in env.Glob(sourceDir.path + "/locale/*/lc_messages/*.po"):

styles = os.path.join(userDocsDir.path, "styles.css")
numberedHeadingsStyle = os.path.join(userDocsDir.path, "numberedHeadings.css")
logo = os.path.join(userDocsDir.path, "favicon.ico")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
logo = os.path.join(userDocsDir.path, "favicon.ico")
logo = os.path.join(sourceDir.path, "images", "nvda.ico")

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.

Add page icon to documentation
2 participants