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

Design GitHub source code links #518

Closed
Eric-Arellano opened this issue Dec 12, 2023 · 5 comments
Closed

Design GitHub source code links #518

Eric-Arellano opened this issue Dec 12, 2023 · 5 comments

Comments

@Eric-Arellano
Copy link
Collaborator

Eric-Arellano commented Dec 12, 2023

This ticket is to track the visual design of #454.

Current implementation

We simply use an external link on the same line as the code object's description. It always appears at the end of the code and is normal text - we don't do any funky floating or anything.

Screenshot 2024-01-11 at 4 49 15 PM

I suspect this design is probably fine other than weirdness with the padding. Note how the underline extends directly to the code. I couldn't fix this by adding extra whitespace in the content - it wouldn't work. I think we need to adjust the CSS.

Screenshot 2024-01-11 at 4 50 32 PM

But what is tricky about adjusting the CSS is it will impact all inline code highlighting:

Screenshot 2024-01-11 at 4 51 25 PM

Note that we're using padding-right for inline code, rather than margin-right, which would result in the background not being the shaded gray.

Original notes

What the old qiskit.org design looks like

Screenshot 2023-12-12 at 12 46 18 PM

When you click the [source] link, it takes you a new webpage on qiskit.org with a copy of the source code:

Screenshot 2023-12-12 at 12 46 54 PM

Our implementation will instead take you to GitHub.com, so be an external link.

Note how the old design handles the # icon to get the permalink for the current code object. This only happens on hover:

Screenshot 2023-12-12 at 12 47 51 PM

Which code objects have links?

Note that not every single code object has a link, For example, in the first image in this issue, only exception QiskitNatureError has it, but not its method with_traceback().

However, there are other places where methods do have their own link. (Methods are a lower information hierarchy than the class they belong to.)

Screenshot 2023-12-12 at 12 49 35 PM

So, I think the safest thing to design for is that both the top-level code object—like the class—and also the code objects one level lower—like methods—can have the link but they might not.

--

The link should probably show up on the same line as the name of the code object itself? But, it is common that the line will wrap:

Screenshot 2023-12-12 at 12 54 20 PM

Copy for the link

We discussed in a meeting that [source] might be vague. We only used that name with qiskit.org because it's what Sphinx itself uses and we can't easily change it.

For the new Qiskit docs, maybe we want something more explicit like [GitHub]? Or GitHub ↗?

@Eric-Arellano Eric-Arellano changed the title Design GItHub source code links Design GitHub source code links Dec 12, 2023
@Eric-Arellano Eric-Arellano added this to the 24-02-13 Qiskit 1.0 milestone Dec 12, 2023
Eric-Arellano added a commit that referenced this issue Jan 11, 2024
Implements #454 for
projects that are using `sphinx.ext.viewcode`. All 3 of our projects
were historically using it until I turned it off after removing
qiskit.org, so the historical API docs are good to go. I'll restore it
for current versions of these projects and regenerate the docs.

This implementation is not a perfect implementation:

1. The visual design is a little awkward, especially the lack of
padding. This can be improved via
#518

<img width="571" alt="Screenshot 2024-01-11 at 12 28 07 PM"
src="https://github.com/Qiskit/documentation/assets/14852634/2d915b94-ec48-445a-a174-ace628655b4b">

2. The link only takes you to the overall code page, not the specific
lines. This could be improved via
#517.

But it's good enough to not block on these improvements.

This PR regenerates all Runtime historical versions, but not any current
versions nor Qiskit historical versions.

## How source code URLs are determined

`sphinx.ext.viewcode` embeds a copy of every Python file used in API
docs and uses internal relative links like
`../modules/qiskit_ibm_runtime/ibm_backend.html`. They correspond to
Python files we can be confident exist. We transform those relative
links into GitHub links here:


https://github.com/Qiskit/documentation/blob/790e9372f64ab7d5f15eaccc229b4d0765781d44/scripts/lib/api/processHtml.ts#L94-L105


https://github.com/Qiskit/documentation/blob/790e9372f64ab7d5f15eaccc229b4d0765781d44/scripts/lib/api/processHtml.ts#L163-L183

Our links assume that there is a branch called
`stable/<versionWithoutPatch` in GitHub, like `stable/0.8`.

## Replacing `[source]` with `GitHub ↗︎`

We remove the original link from Sphinx and replace it with our own.
This is important so that the link is not included in the `<code>` HTML
element incorrectly. It also allows us to set a custom link label and
`title` (the text when highlighting).
@Eric-Arellano Eric-Arellano removed this from the 24-02-13 Qiskit 1.0 milestone Jan 11, 2024
@gracelindsell
Copy link
Collaborator

gracelindsell commented Feb 19, 2024

@Eric-Arellano In the new figma designs, I have updated so the padding is corrected. I think keeping it to Github instead of Source is more clear
github example

@Eric-Arellano
Copy link
Collaborator Author

@gracelindsell yes to that part. My doubt is whether we should be changing the behavior for all code blocks outside of API docs. Look at this screenshot:

image

Should that padding be there for run()? Or should it be a margin? Or less padding/margin either way?

@Eric-Arellano
Copy link
Collaborator Author

We discussed that the padding is too big in general. Rather than 0.5rem which we think is roughly 8px, it should be about 0.25rem which would be 4px. That will impact all inline code blocks.

But independently, we also need to figure out why the GitHub text has no margin between the code block, which differs from other code blocks we have like this run() example.

@gracelindsell
Copy link
Collaborator

@Eric-Arellano Right, any text or link that follows a code block should be 8px away from the code block, and thus 12px away from the text in the code block

@Eric-Arellano
Copy link
Collaborator Author

Closing this in favor of #856. Thanks, Grace!

frankharkins pushed a commit to frankharkins/documentation that referenced this issue Jul 22, 2024
Implements Qiskit#454 for
projects that are using `sphinx.ext.viewcode`. All 3 of our projects
were historically using it until I turned it off after removing
qiskit.org, so the historical API docs are good to go. I'll restore it
for current versions of these projects and regenerate the docs.

This implementation is not a perfect implementation:

1. The visual design is a little awkward, especially the lack of
padding. This can be improved via
Qiskit#518

<img width="571" alt="Screenshot 2024-01-11 at 12 28 07 PM"
src="https://github.com/Qiskit/documentation/assets/14852634/2d915b94-ec48-445a-a174-ace628655b4b">

2. The link only takes you to the overall code page, not the specific
lines. This could be improved via
Qiskit#517.

But it's good enough to not block on these improvements.

This PR regenerates all Runtime historical versions, but not any current
versions nor Qiskit historical versions.

## How source code URLs are determined

`sphinx.ext.viewcode` embeds a copy of every Python file used in API
docs and uses internal relative links like
`../modules/qiskit_ibm_runtime/ibm_backend.html`. They correspond to
Python files we can be confident exist. We transform those relative
links into GitHub links here:


https://github.com/Qiskit/documentation/blob/486afd9b16be52dfedf63cdd357708884c99e110/scripts/lib/api/processHtml.ts#L94-L105


https://github.com/Qiskit/documentation/blob/486afd9b16be52dfedf63cdd357708884c99e110/scripts/lib/api/processHtml.ts#L163-L183

Our links assume that there is a branch called
`stable/<versionWithoutPatch` in GitHub, like `stable/0.8`.

## Replacing `[source]` with `GitHub ↗︎`

We remove the original link from Sphinx and replace it with our own.
This is important so that the link is not included in the `<code>` HTML
element incorrectly. It also allows us to set a custom link label and
`title` (the text when highlighting).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

3 participants