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

Bug/3474 #3647

Merged
merged 6 commits into from Sep 29, 2017
Merged

Bug/3474 #3647

merged 6 commits into from Sep 29, 2017

Conversation

JoshuaWhatley
Copy link
Contributor

Fixing #3474

This styling issue is more prevalent on smaller devices and/or when the operation paths are very long:

Current petstore:
image

New petstore:
image

@webron webron requested a review from shockey September 11, 2017 08:59
@@ -251,9 +251,11 @@
{
font-size: 16px;

display: flex;
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason why display: flex was removed? AFAIK, the flex property doesn't implicitly trigger display: flex on the element.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The flex context is enabled via its direct parent and I didn't see a reason for the context to be passed further down to its children. So I removed it as unnecessary. It does no harm to keep it, if you foresee a future need for it.

JoshuaWhatley and others added 3 commits September 13, 2017 21:23
Changed "break-word" to "break-all" as this is standard across all
browsers and results with consistent behavior.
Copy link
Contributor

@shockey shockey left a comment

Choose a reason for hiding this comment

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

Tested - the changes look better than the current baseline. Still not great (true responsiveness is something we definitely don't have - keep in mind we officially don't support below ~768px width today), but better!

Thanks @JoshuaWhatley!

@shockey shockey merged commit ca98ed1 into swagger-api:master Sep 29, 2017
@JoshuaWhatley
Copy link
Contributor Author

No problem, happy to help! I didn't know about the ~768px minimum. I will keep that in mind moving forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants