Repository navigation
Add a viewport and make the two over-wide layouts usable on a phone - #36
Merged
Merged
Conversation
The layout emitted no viewport meta tag, so a phone rendered every page at a ~980px virtual width and scaled it down: width-based CSS keyed off a width the device does not have. Added width=device-width, initial-scale=1, plus lang="en" on <html>, which the page never declared. Two layouts genuinely do not fit. The nav bar is generated from $_models: 19 tabs at 120px = 2280px, against a #navbar ul that sets overflow:hidden, so tabs past the edge are clipped rather than wrapped -- an admin sees 10 of them. The alert queue is display:table with fixed table-column widths summing to 1140px. Plus 16 index views using a plain <table>, and dashboard panels floated 49% two-up. All narrow-screen rules go inside one @media (max-width: 768px) block, so desktop rendering is unchanged by construction rather than by inspection. That matters because the rack-test suite has no layout engine and cannot validate responsive CSS. Verified mechanically: split the compiled bundle at the media query and the portion outside it is byte-identical to the previous build (8888 bytes). Tables are handled by one CSS rule rather than 16 templates. white-space: nowrap is deliberately left out -- several tables have a description column and nowrap would force one very long line per row. No hamburger menu: that needs JS the app does not have. Wrapping the tabs at 33% makes all 19 reachable with no new behaviour. test/system/responsive_test.rb guards the containment (nothing may follow the @media block) and the viewport tag. Both checked against deliberate breakage: a leaked rule fails the first, deleting the tag fails the second. Co-authored-by: openhands <openhands@all-hands.dev>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds the missing
viewportmeta tag, and makes the two layouts that cannot fit a phone usable at narrow widths.The tag
The layout emitted no
viewportmeta tag. Without it a phone renders the page at a ~980px virtual width and scales the result down - so any width-based CSS keys off a width the device does not have. Addedwidth=device-width, initial-scale=1. Also addedlang="en"to<html>, which the page had never declared.Two layouts genuinely do not fit - and the numbers say so
#navbar ulsetsoverflow: hidden, so tabs past the edge are not wrapped and not reachable - they are clipped. An admin sees 10 of 19 tabs.display:tablewith fixeddisplay:table-columnwidths. No phone is that wide.Plus 16 index views using a plain
<table>, and dashboard panels floated at 49% two-up.The nav bar width is not a fixed number I picked: it is generated from
$_models, which currently holds 19 models. Adding a model adds a nav tab.Why this is verifiable without a browser
All narrow-screen rules go inside a single
@media (max-width: 768px)block. That makes desktop rendering unchanged by construction rather than by inspection - which matters, because the rack-test system suite has no layout engine and cannot tell whether responsive CSS is correct.Proven mechanically rather than asserted: the compiled bundle was split at the media query, and the portion outside it compared byte-for-byte against the previous build.
The media query sits mid-bundle (byte 3249 of 9733), ahead of the CodeMirror section, and collides with none of its selectors.
Tables: one rule, not 16 templates
table { display: block; overflow-x: auto }keeps any sideways scroll inside the table rather than the page, and leavesthead/tbody/tr/tdat their defaulttable-*display so row structure is untouched.white-space: nowrap- the third declaration this pattern usually carries - is deliberately omitted. Several of these tables have a description column, and nowrap forces one very long line per row, so it can produce a table far wider than the one it was meant to tame. Letting cells wrap cannot. The proper narrow-screen treatment (stacking each row into labelled blocks) needs adata-labelon every cell, so it belongs with the view de-duplication rather than here.No hamburger menu
Making the nav bar collapsible needs JS the app does not have. Letting the tabs wrap at 33% width makes all 19 reachable with no new behaviour - the smaller and more honest fix.
Guards added
The containment is the whole safety argument, so it is asserted in
test/system/responsive_test.rb:width=device-width@mediablock - a rule that falls out of it applies at every width and silently changes desktop renderingBoth checked against deliberate breakage:
#leaked { color: red; }after the blockExpected "#leaked { color: red; }" to be empty.- failsVerification
rake testrake test:systemscript/chec 8000 k_asset_paths.rbDocs updated:
RECOMMENDATIONS.mdsection 5.12, andAGENTS.mdrecords the containment invariant plus the nav-bar/alert-queue widths so a future change does not undo them.This PR was created by an AI agent (OpenHands) on behalf of the user.