-
Notifications
You must be signed in to change notification settings - Fork 9k
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(styling): Make the Swagger UI Operation page more responsive on smaller screen #9325
Fix(styling): Make the Swagger UI Operation page more responsive on smaller screen #9325
Conversation
7e32234
to
6524fe4
Compare
aa1dc8f
to
7001058
Compare
7001058
to
edcd862
Compare
Hi @pedoch, thank you for the PR. I've tested locally and it works well. The only noticeable negative effect is that the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@AlexanderKhudoev would you mind issuing a PR? |
I unfortunately don't have time to do that yet. I don't know anyone who uses Swagger on mobile devices. In my practice for 10 years, everyone has only used Swagger through the Desktop browser. And now, having moved the Authorize button to the left, it's become awkward to use Swagger on Desktops. |
Apologies, I didn't notice this when testing. @char0n I can push a fix for it if given a couple of hours. |
@AlexanderKhudoev it turns out the reason the button was on the left in your screenshot was because the Schemes selector was not present. I failed to take that into account as it was always present while I was working on the issue. @char0n I have now raised this PR: #9387 which takes this into account and ensures the Authorize button is always on the right on larger screens. Please feel free to take a look at it and approve/merge if you think the proposed solution makes sense. |
Thank you both @AlexanderKhudoev and @pedoch for early regression report and fix. |
Hey there @pedoch , while this is a few months after this has gone in, I believe the changes in this PR introduced a regression around setting E.x. on version 5.9.4 (after this change went in): And on version 5.9.3 (before this change went in): I haven't seen a fix go up yet for this anywhere, so just wanted to call it out at least. |
Hi @bshain-vtinfo, Thanks a lot for the report. Would you try to investigate and issue a fixing PR? We would really appreciate it. |
Make the Swagger UI page more responsive on smaller screens.
Description
This PR does the following:
550px
, when the screen is too small to contain both on the same line. [Screenshot section 1]Motivation and Context
These changes will improve the responsiveness of Swagger UI on smaller screens. It started off as a fix for this issue - #8940 but grew to a general responsiveness improvement.
Resolves #8940
How Has This Been Tested?
I tested these style changes using multiple devices on multiple platforms with different screen sizes.
Also, because these are just styling changes no new tests are needed and all existing tests pass.
Screenshots (if appropriate):
[1] TopBar Screenshot
Before:
After:
[2] Authorize Button Screenshot
Before:
After:
[3] Operation Summary Screenshots
Before:
After:
Checklist
My PR contains...
src/
is unmodified: changes to documentation, CI, metadata, etc.)package.json
)My changes...
Documentation
Automated tests