fixed mobile-responsiveness-ui #745
Conversation
|
|
WalkthroughRemoved inline JavaScript that enforced a fixed 500px layout constraint on the API page, replacing it with flexible, responsive CSS rules that prevent overflow, enable horizontal scrolling, and improve layout safeguards across the documentation site. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
docs/stylesheets/extra.css (3)
338-347: Reconsider universal selector on.swagger-ui *withmax-width: 100%.Using a universal selector (
.swagger-ui *) to applymax-width: 100%can have unintended cascade effects on deeply nested elements, potentially constraining internal layout components that should not be restricted. This broad rule might conflict with Swagger UI's internal structure and component sizing logic.Consider scoping the rule more narrowly to specific child elements that actually need the constraint, such as text, code blocks, and tables within Swagger UI.
-.swagger-ui, -.swagger-ui * { - box-sizing: border-box; - max-width: 100%; -} +.swagger-ui { + box-sizing: border-box; +} + +.swagger-ui code, +.swagger-ui pre, +.swagger-ui table { + box-sizing: border-box; + max-width: 100%; +}
37-43: Verify necessity of!importanton.md-header__title.Line 37 uses
!importantonmargin-left, which is generally a code smell. If the base Material theme or other stylesheets are applying conflicting styles, that's worth investigating. Consider whether the specificity of your selectors is sufficient without!important, or if the issue lies upstream.If
!importantis necessary due to unavoidable conflicts in the base theme, add a brief comment explaining why.
176-193: Reduce duplication in heading and text wrapping rules across media queries.Lines 176–193 in the
@media (max-width: 768px)block define heading and paragraph wrapping rules that are partially repeated in the@media (max-width: 480px)block (lines 252–271 with slightly different font sizes). Extract common wrapping properties to a shared rule to reduce duplication and improve maintainability.+ /* Shared heading and text wrapping for small screens */ + @media (max-width: 768px) { + .md-typeset h1, + .md-typeset h2, + .md-typeset h3, + .md-typeset h4, + .md-typeset h5, + .md-typeset h6, + .md-typeset p, + .md-typeset li, + .md-typeset blockquote { + word-wrap: break-word; + overflow-wrap: break-word; + max-width: 100%; + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/backend/backend_python/api.md(0 hunks)docs/stylesheets/extra.css(5 hunks)
💤 Files with no reviewable changes (1)
- docs/backend/backend_python/api.md
🔇 Additional comments (3)
docs/stylesheets/extra.css (3)
330-336: Approve.api-500-wrapperresponsive container.The new
.api-500-wrapperclass replaces the removed fixed 500px layout constraint with a flexible, scrollable container design. This is a solid approach to maintaining backward compatibility while enabling responsive behavior. The inclusion of-webkit-overflow-scrolling: touch;is a good touch for iOS momentum scrolling.
288-324: Approve homepage 4-column responsive layout fix.The selector targeting inline
display:flexstyles and the cascading media queries (900px → 50% width, 540px → 100% width) are a pragmatic solution to handle the inline HTML layout in docs/index.md. Using!importantis appropriate here since you're overriding inline styles. The responsive stacking is well thought out.
432-469: Approve Swagger UI responsive typography adjustments.The font-size reductions and method/path styling across breakpoints (768px and 480px) follow a sensible progressive degradation approach. Reducing font sizes on smaller screens and adjusting word-break behavior is appropriate for Swagger UI's typically dense content.
| html { | ||
| overflow-x: hidden; | ||
| max-width: 100vw; | ||
| } | ||
|
|
||
| body { | ||
| overflow-x: hidden; | ||
| max-width: 100vw; | ||
| position: relative; | ||
| } |
There was a problem hiding this comment.
Reconsider using 100vw on html and body elements.
Using 100vw with overflow-x: hidden can paradoxically cause horizontal overflow issues because 100vw includes the vertical scrollbar width, which varies across browsers and devices. This is a known pitfall that can trigger unwanted horizontal scrolling on some viewports.
Consider using 100% instead, or relying on the default behavior without the explicit max-width constraint on these root elements.
html {
overflow-x: hidden;
- max-width: 100vw;
+ max-width: 100%;
}
body {
overflow-x: hidden;
- max-width: 100vw;
+ max-width: 100%;
position: relative;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| html { | |
| overflow-x: hidden; | |
| max-width: 100vw; | |
| } | |
| body { | |
| overflow-x: hidden; | |
| max-width: 100vw; | |
| position: relative; | |
| } | |
| html { | |
| overflow-x: hidden; | |
| max-width: 100%; | |
| } | |
| body { | |
| overflow-x: hidden; | |
| max-width: 100%; | |
| position: relative; | |
| } |
🤖 Prompt for AI Agents
In docs/stylesheets/extra.css around lines 6 to 15, using max-width: 100vw on
html and body combined with overflow-x: hidden can cause horizontal overflow
because 100vw includes the scrollbar width; replace max-width: 100vw with
max-width: 100% (or remove the explicit max-width entirely) to rely on the
natural layout, and keep overflow-x: hidden only if you still need to forcibly
hide horizontal overflow.
| /* Allow header inner area to wrap so items don't overflow */ | ||
| .md-header__inner { | ||
| flex-wrap: wrap; | ||
| flex-wrap: nowrap; | ||
| align-items: center; | ||
| gap: 6px; | ||
| overflow: hidden; | ||
| max-width: 100%; | ||
| } |
There was a problem hiding this comment.
Fix contradictory flex-wrap value in comment.
Line 134's comment states "Allow header inner area to wrap," but line 136 explicitly sets flex-wrap: nowrap;. This contradiction suggests either the comment or the code is incorrect and may lead to unintended layout behavior.
If the intent is to allow wrapping, change nowrap to wrap. If the intent is to prevent wrapping, update the comment to reflect that.
/* Allow header inner area to wrap so items don't overflow */
.md-header__inner {
- flex-wrap: nowrap;
+ flex-wrap: wrap;
align-items: center;
gap: 6px;
overflow: hidden;
max-width: 100%;
}🤖 Prompt for AI Agents
In docs/stylesheets/extra.css around lines 134 to 141, the comment says "Allow
header inner area to wrap" but the rule sets flex-wrap: nowrap; — reconcile the
contradiction by either changing the CSS to flex-wrap: wrap; if the header
should be allowed to wrap (and verify overflow/max-width behave as intended), or
update the comment to accurately state that wrapping is prevented (e.g.,
"Prevent header inner area from wrapping") if nowrap is the intended behavior.
|
@Nitinn12 Please link the corresponding issue to this PR. |
2 participants
The PictoPy documentation website had responsiveness issues on smaller screen sizes. On mobile and narrow viewports, content overflowed horizontally, causing important UI elements and text to be partially hidden. This made the documentation difficult to read and navigate.
This PR resolves these issues by improving the responsive layout, ensuring the documentation renders correctly across all screen sizes, including mobile devices.
Changes Made
1.Updated responsive CSS styles
2.Improved layout handling for narrow viewports
3.Ensured consistent spacing and alignment across breakpoints
Summary by CodeRabbit
Documentation
Style
✏️ Tip: You can customize this high-level summary in your review settings.