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 light mode in the doc by default #8100

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sloriot
Copy link
Member

@sloriot sloriot commented Mar 27, 2024

No description provided.

@@ -1,6 +1,6 @@
<!-- HTML header for doxygen 1.9.6-->
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "https://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml" lang="$langISO">
<html xmlns="http://www.w3.org/1999/xhtml" lang="$langISO" class="light-mode">
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the intention here different from the setting HTML_COLORSTYLE=LIGHT? Wouldn't it be much better to set the doxygen setting HTML_COLORSTYLE to LIGHT ?

From the doxygen Documentation:

HTML_COLORSTYLE

The HTML_COLORSTYLE tag can be used to specify if the generated HTML output should
be rendered with a dark or light theme.
Possible values are: LIGHT always generate light mode output, DARK always generate dark mode output,
AUTO_LIGHT automatically set the mode according to the user preference, use light mode if no preference
is set (the default), AUTO_DARK automatically set the mode according to the user preference, use dark mode
if no preference is set and TOGGLE allow to user to switch between light and dark mode via a button.
The default value is: AUTO_LIGHT.

Copy link
Member Author

Choose a reason for hiding this comment

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

I did not check but since we have custom headers this option will not be taken into account.

Copy link
Contributor

Choose a reason for hiding this comment

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

The fact that CGAL uses custom headers is complicating things a bit, I still think that it will be a good idea to set:

HTML_COLORSTYLE        = LIGHT

as in that case some default files will be adjusted as well and might prevent some problems in the future.

I did a quick test with the Manual directory and e.g. in the file dynsections.js we see the differences:

127,128c127,128
<   plusImg:  [ "var(--fold-plus-image)",  "var(--fold-plus-image-relpath)" ],
<   minusImg: [ "var(--fold-minus-image)", "var(--fold-minus-image-relpath)" ],
---
>   plusImg:  [ "url('plus.svg')",  "url('../../plus.svg')" ],
>   minusImg: [ "url('minus.svg')", "url('../../minus.svg')" ],
156c156
<       'background':'linear-gradient(var(--fold-line-color),var(--fold-line-color)) no-repeat 46px/2px 100%'
---
>       'background':'linear-gradient(#808080,#808080) no-repeat 46px/2px 100%'

(lines < are without the setting HTML_COLORSTYLE = LIGHT).

Other files with differences are in .css file:

  • doxygen.css
  • navtree.css
  • search/search.css
  • tabs.css

Not performed changes here might lead to unexpected results in the layout / coloring when var(...) are not replaced.

Copy link
Contributor

Choose a reason for hiding this comment

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

Besides the mentioned files I didn't find evidence that the light switch was used elsewhere.
I also searched for the word light and found no evidence that it would lead to problems.
So I think the setting HTML_COLORSTYLE = LIGHT can be used, unless you can point to a place where the setting leads to problems and the class="light-mode" doesn't.

@sloriot sloriot added Batch_1 First Batch of PRs under testing Under discussion Tested and removed Under Testing Batch_1 First Batch of PRs under testing labels Apr 1, 2024
@sloriot
Copy link
Member Author

sloriot commented Apr 4, 2024

Successfully tested in CGAL-6.0-Ic-209

@lrineau lrineau assigned lrineau and unassigned lrineau Apr 5, 2024
@lrineau lrineau self-assigned this Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants