Skip to content

🐛 Fix showing fast button as external link in animated terminals in docs - #1912

Merged
tiangolo merged 9 commits into
fastapi:masterfrom
phalberg:master
Aug 3, 2026
Merged

🐛 Fix showing fast button as external link in animated terminals in docs#1912
tiangolo merged 9 commits into
fastapi:masterfrom
phalberg:master

Conversation

@phalberg

@phalberg phalberg commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Pull Request

Discussion: #1908

Description

Problem

fast and restart were marked as hyperlinks, it seems that under first initial load the page would render them as true hyperlinks, adding an icon that indicates to users that it will open a new tab and also not adhering to its styling (which is rather to the top right).
Read more on the discussion page for details.

Solution

  • Removing the hyperlink reference and implemented button elements.
  • Updated CSS to preserve the controls existing appearance as much as possible.

Note on small UI issues:

- Upon scolling down to the bottom of the page, then clicking fast on a terminal widget, the content-side-bar overlaps the bottom content - slightly (if the side-bar is long enough). (I can't reproduce this after 09ede97)

Note: These two problems are quite minor edge-cases, I don't think there is a need to handle them in this PR - as I do not believe this PR is relevant to fix these issues - nonetheless I wanted to mention them as I see them more openly when I make this fix.

AI Disclaimer

I used ChatGPT 5.5 (High) to help investigate the issue. The final solution was determined and implemented manually.

Checklist

  • This PR links to a GitHub Discussion for the proposed code change.
  • I added tests for the change.
  • The new or updated tests fail on the main branch and pass on this PR.
  • Coverage stays at 100%.
  • The documentation explains the change if needed.

@github-actions github-actions Bot added the docs Improvements or additions to documentation label Jul 28, 2026
@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

@phalberg

phalberg commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

To have same functionality as a button we want a hover:

  • Add hover functionality (f365d64 should handle this case. )

@phalberg

phalberg commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Ready from my side, the solution seems to have fixed the major problems!

Note

These "elements" fast and restart should perhaps be handled as true buttons sometime, but I will leave that for now, to just fix the current issues in this PR.

@phalberg
phalberg marked this pull request as ready for review July 28, 2026 08:11
@phalberg
phalberg marked this pull request as draft July 28, 2026 08:23
@phalberg
phalberg marked this pull request as ready for review July 28, 2026 08:40
@YuriiMotov

Copy link
Copy Markdown
Member

These "elements" fast and restart should perhaps be handled as true buttons sometime, but I will leave that for now, to just fix the current issues in this PR.

@phalberg, have you evaluated how mush changes would introduce turning them into real buttons?

@phalberg

Copy link
Copy Markdown
Contributor Author

These "elements" fast and restart should perhaps be handled as true buttons sometime, but I will leave that for now, to just fix the current issues in this PR.

@phalberg, have you evaluated how mush changes would introduce turning them into real buttons?

Hey @YuriiMotov, thanks for asking.

I haven’t evaluated the full impact of that change yet, but I can take a closer look and see if there are side-effects, etc then report back with what I find.

@YuriiMotov

Copy link
Copy Markdown
Member

I haven’t evaluated the full impact of that change yet, but I can take a closer look and see if there are side-effects, etc then report back with what I find.

That would be great! Thanks!
No need to put much efforts if it requires complex changes, the idea is just to check how difficult it is

@phalberg

phalberg commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

I haven’t evaluated the full impact of that change yet, but I can take a closer look and see if there are side-effects, etc then report back with what I find.

That would be great! Thanks! No need to put much efforts if it requires complex changes, the idea is just to check how difficult it is

No worries!
I got some time tomorrow, so will probably report back then, and have a look.
Thank you.

@phalberg

phalberg commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

These "elements" fast and restart should perhaps be handled as true buttons sometime, but I will leave that for now, to just fix the current issues in this PR.

@phalberg, have you evaluated how mush changes would introduce turning them into real buttons?

Hey again @YuriiMotov.
I evaluated the implementation of buttons and I think its worth changing to so I added it as 09ede97, have a look whenever you guys have a chance. If you are unsure about my implementation or need any adjustments, please let me know, happy to answer and adjust!
The changes were fairly small, and I can no longer reproduce one of the minor UI bugs I had previously. I updated the PR description accordingly.
The biggest win I see in this is that these controls now behave as actual buttons, hopefully preventing future issues related to href.

@YuriiMotov YuriiMotov changed the title Docs: Bug with handling fast button in animated terminals 🐛 Fix showing fast button as external link in animated terminals in docs Jul 30, 2026

@YuriiMotov YuriiMotov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

Suggested a couple of improvements

Comment thread docs/js/termynal.js Outdated
Comment thread docs/js/termynal.js Outdated
Comment thread docs/css/termynal.css
@phalberg

Copy link
Copy Markdown
Contributor Author

LGTM!

Suggested a couple of improvements

Thank you for great suggestions along the way and good help!

@tiangolo tiangolo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Awesome, thank you! 🚀

And thanks @YuriiMotov for all the help. 🙌

@tiangolo
tiangolo merged commit 904a3b2 into fastapi:master Aug 3, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants