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

Move API headings inside the component #1395

Open
Tracked by #479 ...
Eric-Arellano opened this issue May 15, 2024 · 2 comments
Open
Tracked by #479 ...

Move API headings inside the component #1395

Eric-Arellano opened this issue May 15, 2024 · 2 comments

Comments

@Eric-Arellano
Copy link
Collaborator

Eric-Arellano commented May 15, 2024

This is similar in spirit to #948. There, we were proposing getting rid of the header entirely, and we decided to not do it for a few reasons:

  1. We like the visual design of having the header
  2. Our search differentiates between headers vs body, and we want to keep the header as a search entry
  3. The header should show up in the right page ToC (if it's the correct level w/ module pages)
  4. The header makes the anchor link experience consistent with the rest of the platform and simpler to implement, such as avoiding issues with overloaded signatures (semi-related to Fix Furo styling for API pages with methods on class page qiskit_sphinx_theme#458)

Unlike #948, this is not proposing removing headers entirely. Instead, it proposes moving the header inside the code component, and replacing the <h3>-<h6> element with <dl>, <dt>, and <dd> for better HTML semantics.

Before: Screenshot 2024-05-15 at 5 07 03 PM
After (proof-of-concept): Screenshot 2024-05-15 at 5 06 26 PM

Motivation

We noticed in Qiskit/qiskit#12403 (review) that the header living above the code object and its margin-top is resulting in unclear API docs where it is not clear which text describes the code object.

This proposal is meant to make the code component more self-contained, that the entire definition is in one place and contained by the left colored bar.

@jakelishman is also correct in Qiskit/qiskit#12403 (comment) that the HTML semantics would be more appropriate to use <dl>, <dt>, and <dd> rather than <h3> because indeed these code components are giving a single self-contained definition, rather than defining a new heading section that applies to all content beneath it. Improving the HTML modeling better complies with semantic web, which helps accessibility and SEO. (Note that we can still style <dl> however we want!)

Implementation

Styling & levels

We will style the <dt> similar to <h3> - <h6>, although it should have less margin-top.

TBD if the <dt> should render differently based on the surrounding headers. For example, https://docs.quantum.ibm.com/api/qiskit/dev/circuit#bits-and-registers is an h3; should the <dt> look like an <h4> or <h3>?

Option 1: h4 Screenshot 2024-05-15 at 6 11 07 PM
Option 2: h3 Screenshot 2024-05-15 at 6 13 28 PM

For inline classes, what about their methods? Should they still look like a smaller header than their owning class?

example of inline class using h4 and h5 Screenshot 2024-05-15 at 6 15 19 PM

Note that the level we use for styling does not need to be the same as the level we use for the Page ToC! That is unlike the current implementation. This is more flexible.

(Eric's thoughts: we should preserve the current behavior of headers getting smaller based on surrounding context. It visually shows the information hierarchy better. For example, if you have a <Function> inside an h5 section, it is visually confusing to show the Function's header as h3.)

Page ToC

The Page ToC code will need to be modified to read the <dt> from these components. We'll need to know the "level" because we only show h1-h3 on class pages and h2-h3 on module pages, so we should not show any <dt> where the level >3.

Search

We need to teach the search index script (parseDocument.ts) to treat these <dt>s as headers. We should probably always treat <dt> as a header for search - note that we don't ever use <dt> atm.

Anchor tag

We need to preserve this mechanism

Screenshot 2024-05-15 at 6 16 16 PM
@jakelishman
Copy link
Member

Thank you so much for this! I know it's probably not the biggest priority, but it'll be great to have it.

@javabster
Copy link
Collaborator

This change looks sensible to me - as for styling I dont really have strong feelings between option 1 or 2, maybe option 2 is slightly nicer?

@Eric-Arellano Eric-Arellano changed the title Move API headers inside the component Move API headings inside the component Oct 31, 2024
github-merge-queue bot pushed a commit that referenced this issue Nov 1, 2024
Closes #2210. Before,
cross references to inlined class methods would try using the link
`InlineClass.some_method`, but the link should really be `some_method`
without `InlineClass`.

The better fix is #2215
because it avoids the risk of ambiguous anchors. However, it would be a
UX regression to land that until
#1395 is done. Fixing this
is tracked by #2217.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

3 participants