Skip to content

Use <details> in the cgalNParam #6920

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

albert-github
Copy link
Contributor

@albert-github albert-github commented Sep 30, 2022

Since doxygen 1.9.3 doxygen supports the HTML <details> tag, so no difficult constructs necessary with \htmlonly.

cgalNParam is used in 122 files.
Example Tetrahedral_remeshing/group__PkgTetrahedralRemeshingRef.html#gadeec4a90f1b4f8c9bca76eaee9181235

Old

image

New

image

Since doxygen 1.9.3 doxygen supports the HTML `<\details\>` tag, so no difficult constructs necessary with `\htmlonly`.
@MaelRL

This comment was marked as outdated.

@MaelRL MaelRL added Doc Not yet approved The feature or pull-request has not yet been approved. labels Sep 30, 2022
@github-actions

This comment was marked as outdated.

- Improve color etc. of the optional parameter so it looks like a real parameter (using the CSS `class=paramname` for summary)
- make `arrow`s like doxygen treeview arrows
@albert-github
Copy link
Contributor Author

albert-github commented Sep 30, 2022

I've just pushed a slightly improved version (in respect to parameter color and the type of the arrow, all more doxygen like)

New

image

Set proper scaling of the arrow
@sloriot
Copy link
Member

sloriot commented Oct 4, 2022

I don't really like the change. Comparing this (proposed change) to this (current version).

There is an extra empty line and the close/open action was smooth while it would be "non-continuous" with the current proposal. The arrow style is not great too. Maybe this is something we can deal with by looking at the options of details in the doc.

@albert-github
Copy link
Contributor Author

albert-github commented Oct 4, 2022

In the original version the arrows are quite intrusive and don't look like those in the tree view.
This can however be overcome quite easily in the current version by changing in the cgal_styleshee.css for

.collapsible:after {

the lines:

  content: '\25B6';
  color: #7A93C5;
  font-weight: bold;

into

  content: '\25BA';
  font-size: 80%;

(in the past doxygen fiddled around a bit with the the right pointing arrow, shee for the differences between the 25B6 and the 25BA https://www.codetable.net/decimal/9654 and https://www.codetable.net/decimal/9660:

image

image

so just a small difference in width and height)

In the new version there are indeed 2 things:

  • extra empty whitespace
  • the jump versus smooth scroll

I'll see if I can find something, easy, for these 2 problems as I think it is favorable / more consistent to use the <details> version compared to have something with a \htmlonly block / workaround.

@MaelRL
Copy link
Member

MaelRL commented Oct 4, 2022

I don't think the change in arrow shape in the tree view -and a fortiori in the NPs- is actually a good thing.

Also I liked that it had the same blue color, but I understand that this is personal pref.

And nitpicking, but (on the line) the text is too close to the arrow. Even if it's to have symmetry with the tree view, it's a wider space there.

@sloriot
Copy link
Member

sloriot commented Oct 4, 2022

I don't think the change in arrow shape in the tree view -and a fortiori in the NPs- is actually a good thing.

Also I liked that it had the same blue color, but I understand that this is personal pref.

And nitpicking, but (on the line) the text is too close to the arrow. Even if it's to have symmetry with the tree view, it's a wider space there.

I agree I don't see why when it's expended it's a equilateral triangle and when it's collapsed it's an isosceles triangle. My personal opinion would be to update the treeview to use only equilateral triangles.

@albert-github
Copy link
Contributor Author

I don't think the change in arrow shape in the tree view -and a fortiori in the NPs- is actually a good thing.

This is a personal preference, and this would, probably, mean just a small change in the relevant css file for the color and the weight.
I think it is good that both shapes are on one line.

Also I liked that it had the same blue color, but I understand that this is personal pref.

Indeed a personal preference, I like it when it has the same color as the (optional) parameter it refers to as in that case it indicates that there is something for that parameter.

And nitpicking, but (on the line) the text is too close to the arrow. Even if it's to have symmetry with the tree view, it's a wider space there.

This can easily be adjusted currently it is at 4px, but this can easily be changed

@albert-github
Copy link
Contributor Author

#6920 (comment)

As far as I remember well the different browsers had some different opinions about the arrows and this combination was the best we could find.

@albert-github
Copy link
Contributor Author

albert-github commented Oct 5, 2022

Small update:

extra empty whitespace

this looks like a doxygen problem, as there is a paragraph before the <summary> in doxygen, we are looking into this problem.

the jump versus smooth scroll

In the current code a small hack.js function is made to get the smooth scroll in of the text, looking into the problem to obtain this also with the <details> tag (question at stack exchange, no solution yet).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Doc Not yet approved The feature or pull-request has not yet been approved. Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants