-
Notifications
You must be signed in to change notification settings - Fork 85
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
[ EPIC ] Improve API Ref UX/UI #479
Milestone
Comments
This was referenced Dec 12, 2023
github-merge-queue bot
pushed a commit
that referenced
this issue
Dec 21, 2023
Part of fixing #542. ## Methods living on dedicated pages Before Qiskit 0.44, methods lived on dedicated HTML pages. We changed that in Qiskit 0.44 because it resulted in horrible build-time performance when migrating `qiskit.org/documentation` from the old Pytorch theme to the new Furo theme, so that instead methods lived directly on their class pages. It looks like the script `updateApiDocs.ts` was trying to emulate methods living on class pages before Qiskit 0.44 through the file `mergeClassMembers.ts`. This code attempted to remove the method pages and instead have them live directly on the class pages. I believe the original script author had `mergeClassMembers.ts` to try to avoid performance issues. Until a fix a few weeks ago to the docs application, having a huge amount of pages would cause the site to load extremely slowly due to the left side bar being too large. That is now fixed because the left side bar is loaded dynamically rather than statically. ## `mergeClassMembers.ts` was causing problems 1. It looks like it wasn't behaving properly, at least some of the time. We did not exhaustively audit every page, but for example, https://docs.quantum.ibm.com/api/qiskit/0.33/qiskit.result.Result only had properties at the top, then a table summarizing all the methods below, but not the methods actually inlined: <img width="774" alt="Screenshot 2023-12-21 at 11 43 47 AM" src="https://github.com/Qiskit/documentation/assets/14852634/576ff54a-763c-484d-bc3b-42068693b0d1"> 2. We didn't know about this inlining when setting up qiskit.org redirects, which do a 1:1 mapping so that dedicated method pages take you to the dedicated method page on IBM, which was 404ing. ## Solution: allow dedicated method pages By deleting `mergeClassMembers.ts`, the script will properly generate dedicated HTML pages for each method. We took this approach—rather than instead fixing `mergeClassMembers.ts`—for a few reasons: 1. It allows us to avoid messing with our redirect rules, which are the 1:1 mapping. 2. We've gotten user feedback that the API docs are not as usable as they could be. We are planning to fix that in #479, but it won't be fixed in time for putting out the fire of #542. In the meantime, it is more user-friendly to have dedicated method pages for those historical API docs. The biggest reason we would not want to take this approach of dedicated method pages is the possible impact on performance mentioned above. But that has been fixed already in the IBM application. ## Impact of this PR No impact when generating docs where methods already live on class pages, like Qiskit 0.44+. For earlier Qiskit versions, it results in the method pages being generated. Those will be added in follow up PRs. --------- Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
4 tasks
frankharkins
pushed a commit
to frankharkins/documentation
that referenced
this issue
Jul 22, 2024
Part of fixing Qiskit#542. ## Methods living on dedicated pages Before Qiskit 0.44, methods lived on dedicated HTML pages. We changed that in Qiskit 0.44 because it resulted in horrible build-time performance when migrating `qiskit.org/documentation` from the old Pytorch theme to the new Furo theme, so that instead methods lived directly on their class pages. It looks like the script `updateApiDocs.ts` was trying to emulate methods living on class pages before Qiskit 0.44 through the file `mergeClassMembers.ts`. This code attempted to remove the method pages and instead have them live directly on the class pages. I believe the original script author had `mergeClassMembers.ts` to try to avoid performance issues. Until a fix a few weeks ago to the docs application, having a huge amount of pages would cause the site to load extremely slowly due to the left side bar being too large. That is now fixed because the left side bar is loaded dynamically rather than statically. ## `mergeClassMembers.ts` was causing problems 1. It looks like it wasn't behaving properly, at least some of the time. We did not exhaustively audit every page, but for example, https://docs.quantum.ibm.com/api/qiskit/0.33/qiskit.result.Result only had properties at the top, then a table summarizing all the methods below, but not the methods actually inlined: <img width="774" alt="Screenshot 2023-12-21 at 11 43 47 AM" src="https://github.com/Qiskit/documentation/assets/14852634/576ff54a-763c-484d-bc3b-42068693b0d1"> 2. We didn't know about this inlining when setting up qiskit.org redirects, which do a 1:1 mapping so that dedicated method pages take you to the dedicated method page on IBM, which was 404ing. ## Solution: allow dedicated method pages By deleting `mergeClassMembers.ts`, the script will properly generate dedicated HTML pages for each method. We took this approach—rather than instead fixing `mergeClassMembers.ts`—for a few reasons: 1. It allows us to avoid messing with our redirect rules, which are the 1:1 mapping. 2. We've gotten user feedback that the API docs are not as usable as they could be. We are planning to fix that in Qiskit#479, but it won't be fixed in time for putting out the fire of Qiskit#542. In the meantime, it is more user-friendly to have dedicated method pages for those historical API docs. The biggest reason we would not want to take this approach of dedicated method pages is the possible impact on performance mentioned above. But that has been fixed already in the IBM application. ## Impact of this PR No impact when generating docs where methods already live on class pages, like Qiskit 0.44+. For earlier Qiskit versions, it results in the method pages being generated. Those will be added in follow up PRs. --------- Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
Closing in favor of the more focused issue #1808 🚀 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
We've been receiving a lot of feedback about the UX/UI of our API reference pages so I'm tracking them all together as one epic here. Some of these may be due to bugs in the API generation script, others might just be features that we forgot to include that were present in qiskit.org and now users are noticing they've disappeared. Either way they all come under the general umbrella of "things we should address to make the API references better". There may be some issues we could close as duplicates
Information hierarchy / headers
h1
in their page table of contents #932Visual design
class
modifier in front of class signatures #1287Params
andRaises
in API docs #1361Rendering bugs
|
inside table element values #488undefined
spans in API docs #485NoiseAdaptiveLayout
#1200qiskit.providers.BackendV1
#1199The text was updated successfully, but these errors were encountered: