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 Furo styling for API pages with methods on class page #458

Merged
merged 7 commits into from
Jul 11, 2023

Conversation

Eric-Arellano
Copy link
Collaborator

@Eric-Arellano Eric-Arellano commented Jul 3, 2023

Background

We're planning to switch some Qiskit projects to stop having dedicated HTML pages per method, mostly due to performance concerns.

Styling changes

Closes #466. Methods were using inline-block rather than block when on the classes page, which resulted in a bug when highlighting over the method to show the # anchor tag:

Screenshot 2023-07-07 at 11 25 45 AM

Instead, we now turn off floating of the [source] label because it causes issues on long code signatures:

Screenshot 2023-07-07 at 3 02 44 PM

After:

Screenshot 2023-07-07 at 3 03 08 PM

Even though the [source] floating to the right is more "elegant", we have too many long signatures in Qiskit to be worth it. It's safer to turn off the float.

This also improves the information hierarchy so that page feels less crowded. Relates to #459:

  • Use a border-width of 2px for top-level code objects, but otherwise 1
  • Increases the font sizes so headings look bigger.

@Eric-Arellano Eric-Arellano changed the title [experiment] Change autosummary template Fix Furo styling for API pages with methods on class page Jul 7, 2023
@Eric-Arellano Eric-Arellano marked this pull request as ready for review July 7, 2023 17:29
@javabster
Copy link
Collaborator

oof yeah these pages are a lot busier now with all these method docs strings. We definitely need to rethink from a design perspective how to make these pages generally feel less overwhelming (I think @frankharkins was planning on doing some work on this?). In the meantime, how do we feel about using 2px purple lines for h2 level titles and 1px purple lines for h3?

Copy link
Collaborator

@javabster javabster left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

At a later date we should still redesign this whole api ref experience, but for now I think it is ok

@@ -76,7 +94,7 @@ dl[class]:not(.option-list):not(.field-list):not(.footnote):not(.glossary):not(.
// section headers like "Attributes" and "Methods". We disagree with the design in
// https://github.com/pradyunsg/furo/discussions/505. Instead, use the rules from `p.rubric`.
dl.py dd p.rubric {
font-size: 1.125em;
font-size: 125%; // We make this bigger than p.rubric to match the bigger font sizes for the page.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have a feeling that using percentage for font size is not best practice and it's better to use rem, but not confident. I think it depends if we want the size to be relative to the parent element or relative to the root element

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I went with percentage because I realized it's what Furo itself is using. So be consistent.

@Eric-Arellano Eric-Arellano merged commit 43d7442 into main Jul 11, 2023
@Eric-Arellano Eric-Arellano deleted the EA/change-autodoc branch July 11, 2023 18:35
Eric-Arellano added a commit to Eric-Arellano/qiskit_sphinx_theme that referenced this pull request Jul 11, 2023
We're planning to switch some Qiskit projects to stop having dedicated
HTML pages per method, mostly due to performance concerns.

Closes Qiskit#466. Methods
were using `inline-block` rather than `block` when on the classes page,
which resulted in a bug when highlighting over the method to show the
`#` anchor tag:

![Screenshot 2023-07-07 at 11 25 45
AM](https://github.com/Qiskit/qiskit_sphinx_theme/assets/14852634/004da4c0-3702-4cd7-9601-5baa46a992a7)

Instead, we now turn off floating of the `[source]` label because it
causes issues on long code signatures:

![Screenshot 2023-07-07 at 3 02 44
PM](https://github.com/Qiskit/qiskit_sphinx_theme/assets/14852634/19c45f5a-8fc7-44f5-a9d8-997b9a0bfa6e)

After:

![Screenshot 2023-07-07 at 3 03 08
PM](https://github.com/Qiskit/qiskit_sphinx_theme/assets/14852634/55e865c9-b376-40e0-846d-cfdd4b1efd1a)

Even though the `[source]` floating to the right is more "elegant", we
have too many long signatures in Qiskit to be worth it. It's safer to
turn off the float.

This also improves the information hierarchy so that page feels less
crowded. Relates to
Qiskit#459:

* Use a border-width of 2px for top-level code objects, but otherwise 1
* Increases the font sizes so headings look bigger.
Eric-Arellano added a commit that referenced this pull request Jul 11, 2023
…k of #458) (#478)

We're planning to switch some Qiskit projects to stop having dedicated
HTML pages per method, mostly due to performance concerns.

Closes #466. Methods
were using `inline-block` rather than `block` when on the classes page,
which resulted in a bug when highlighting over the method to show the
`#` anchor tag:

![Screenshot 2023-07-07 at 11 25 45

AM](https://github.com/Qiskit/qiskit_sphinx_theme/assets/14852634/004da4c0-3702-4cd7-9601-5baa46a992a7)

Instead, we now turn off floating of the `[source]` label because it
causes issues on long code signatures:

![Screenshot 2023-07-07 at 3 02 44

PM](https://github.com/Qiskit/qiskit_sphinx_theme/assets/14852634/19c45f5a-8fc7-44f5-a9d8-997b9a0bfa6e)

After:

![Screenshot 2023-07-07 at 3 03 08

PM](https://github.com/Qiskit/qiskit_sphinx_theme/assets/14852634/55e865c9-b376-40e0-846d-cfdd4b1efd1a)

Even though the `[source]` floating to the right is more "elegant", we
have too many long signatures in Qiskit to be worth it. It's safer to
turn off the float.

This also improves the information hierarchy so that page feels less
crowded. Relates to
#459:

* Use a border-width of 2px for top-level code objects, but otherwise 1
* Increases the font sizes so headings look bigger.
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.

Furo looks bad with methods on same page as class
2 participants