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

fix nav-bar text and links #56

Merged
merged 7 commits into from
May 27, 2022
Merged

fix nav-bar text and links #56

merged 7 commits into from
May 27, 2022

Conversation

Revathyvenugopal162
Copy link
Contributor

fix #55 wrong nav-bar text and link colors

@Revathyvenugopal162 Revathyvenugopal162 added the bug Defects or glitches reported by users or developers label May 27, 2022
Copy link
Member

@jorgepiloto jorgepiloto left a comment

Choose a reason for hiding this comment

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

Other things related to this:

  • Create a CSS_FILENAME = "paynsys_sphinx_theme.css" constant at the top of this file.

  • Make the get_html_theme_path return a Path instance and not a list. Add docstrings to this function too.

  • Use theme_css_path = theme_path / f"static/css/{CSS_FILENAME} instead of the os.path.join method.

Comment on lines 31 to 32
if not os.path.isfile(theme_css_path):
raise FileNotFoundError(f"Unable to locate pyansys-sphinx theme at {theme_css_path}")
Copy link
Member

Choose a reason for hiding this comment

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

These lines are important, as they check if the CSS file exists.

What I am surprised is the fact that, if our CSS is not being loaded as you said, then why this logic did not trigger?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is because, i think the file is loading in to static/ css when we are doing add_css_file. but we are putting the full path make it to load static/ css / html_path which we are adding

Copy link
Member

Choose a reason for hiding this comment

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

@jorgepiloto
Copy link
Member

jorgepiloto commented May 27, 2022

Steps to solve for this:

  • Implement a variable named CSS_FIELNAME = "pyansys_sphinx_theme.css" at the top.
  • The get_html_theme_path function must return a pathlib.Path instance and not a list.
  • Declare the variable theme_css_path = theme_path / f"static/css/{CSS_FILENAME}.
  • Use the .relative_to() method to properly to get the relative path to the _static directory.

Co-authored-by: Alex Kaszynski <akascap@gmail.com>
Co-authored-by: Alex Kaszynski <akascap@gmail.com>
Copy link
Contributor

@akaszynski akaszynski left a comment

Choose a reason for hiding this comment

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

We're mixing the usage of os and pathlib.Path right now, but this is good to go as is. We can refactor later.

Please incorporate my suggestions.

Revathyvenugopal162 and others added 2 commits May 27, 2022 19:19
Co-authored-by: Alex Kaszynski <akascap@gmail.com>
@Revathyvenugopal162 Revathyvenugopal162 merged commit bcf9b07 into main May 27, 2022
@Revathyvenugopal162 Revathyvenugopal162 deleted the fix/nav_bar_text branch May 27, 2022 17:25
Revathyvenugopal162 added a commit that referenced this pull request May 27, 2022
* fix nav-bar text and links

* add css path

* add css path using pathlib

* Update src/pyansys_sphinx_theme/__init__.py

Co-authored-by: Alex Kaszynski <akascap@gmail.com>

* Update theme path

Co-authored-by: Alex Kaszynski <akascap@gmail.com>

* Update path using pathlib

Co-authored-by: Alex Kaszynski <akascap@gmail.com>

* update path using pathlib

Co-authored-by: Alex Kaszynski <akascap@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Defects or glitches reported by users or developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong navbar text and link colors
3 participants