-
Notifications
You must be signed in to change notification settings - Fork 4
Finished mobile view debugging (except orange ticket issues) #499
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
base: main
Are you sure you want to change the base?
Changes from all commits
56e5186
f1acf5e
4295901
b0ec6db
b111287
7917682
a0abaef
3b6bec5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a way you could deduplicate some of the styling in this file?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed some stylings that are handled at the container level like for tables, but I left the width and maxwidth styles to maintain the sizing more explicitly incase on wordpress it might send in something different. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import { style } from "@vanilla-extract/css" | ||
| import { globalStyle, style } from "@vanilla-extract/css" | ||
| import { hspace, mediaQueries } from "src/style/constants" | ||
| import { paddingX, paddingY } from "src/style/utils" | ||
| import { std } from "src/style/utils.css" | ||
|
|
@@ -13,3 +13,93 @@ export const docHeader = style([ | |
| }, | ||
| }, | ||
| ]) | ||
|
|
||
| export const wordpressContentFix = style({ | ||
| width: "100%", | ||
| maxWidth: "100%", | ||
| overflow: "hidden", | ||
| overflowX: "auto", | ||
| WebkitOverflowScrolling: "touch", | ||
| }) | ||
|
|
||
| // Use globalStyle for child element targeting: | ||
| globalStyle( | ||
| `${wordpressContentFix} h1, ${wordpressContentFix} h2, ${wordpressContentFix} h3, ${wordpressContentFix} h4, ${wordpressContentFix} h5, ${wordpressContentFix} h6`, | ||
| { | ||
| clear: "both", | ||
| width: "100%", | ||
| float: "none", | ||
| display: "block", | ||
| marginBottom: "1rem", | ||
| } | ||
| ) | ||
|
|
||
| globalStyle(`${wordpressContentFix} p, ${wordpressContentFix} div`, { | ||
| clear: "both", | ||
| width: "100%", | ||
| float: "none", | ||
| maxWidth: "100%", | ||
| paddingLeft: "min(var(--padding-left, 0px), 2rem) !important", | ||
| paddingRight: "min(var(--padding-right, 0px), 2rem) !important", | ||
| }) | ||
|
|
||
| globalStyle(`${wordpressContentFix} img`, { | ||
| maxWidth: "100%", | ||
| height: "auto", | ||
| float: "none", | ||
| display: "block", | ||
| margin: "0 auto 1rem auto", | ||
| }) | ||
|
|
||
| globalStyle(`${wordpressContentFix} table`, { | ||
| width: "100%", | ||
| maxWidth: "100%", | ||
| tableLayout: "auto", | ||
| wordWrap: "break-word", | ||
| fontSize: "0.8rem", | ||
| }) | ||
|
|
||
| globalStyle( | ||
| `${wordpressContentFix} table td, ${wordpressContentFix} table th`, | ||
| { | ||
| padding: "0.4rem 0.2rem", | ||
| overflow: "hidden", | ||
| verticalAlign: "top", | ||
| width: "auto", | ||
| minWidth: "0px", | ||
| maxWidth: "calc(50vw - 2rem)", | ||
| } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could change the fixed maxWidth to a variable or theme token so it’s easier to tweak later
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I changed this to half the width of the screen. |
||
| ) | ||
|
|
||
| globalStyle(`${wordpressContentFix} table th`, { | ||
| fontWeight: "bold", | ||
| }) | ||
|
|
||
| globalStyle(`${wordpressContentFix} table td`, { | ||
| lineHeight: "1.2", | ||
| }) | ||
|
|
||
| globalStyle(`${wordpressContentFix} table td img`, { | ||
| width: "auto !important", // Override inline width="300" | ||
| height: "auto !important", // Override inline height="300" | ||
| minWidth: "calc(25vw) !important", | ||
| display: "inline-block !important", | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The 120px minWidth could cause layout issues on small screens. Have we tested this on smaller devices?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I changed this to quarter width of the screens so that it stays more consistent. Though if an image really is too small for a particular device then it probably makes sense for the user to click to enlarge anyways. |
||
| verticalAlign: "top !important", | ||
| }) | ||
|
|
||
| // Specific rules for tables with many columns (eg. 5 or more) making them scrollable | ||
| // Compromise between readability and usability | ||
| globalStyle(`${wordpressContentFix} table:has(tr td:nth-child(5))`, { | ||
| minWidth: "max-content", // Wide tables can go as wide as needed | ||
| whiteSpace: "nowrap", | ||
| }) | ||
|
|
||
| globalStyle(`${wordpressContentFix} pre`, { | ||
| whiteSpace: "pre-wrap", | ||
| wordWrap: "break-word", | ||
| overflowX: "auto", | ||
| maxWidth: "100%", | ||
| margin: "0 0 1rem 0", | ||
| padding: "0.5rem", | ||
| fontSize: "0.85rem", | ||
| }) | ||
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.
I really like the click-to-enlarge feature for images! We could also consider adding an accessibility note (like alt text or a tooltip) so users know the image can be enlarged?
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.
I just added a
titletag saying "click to view full size". I thought about adding a caption that only shows on mobile but I don't think that's possible from the code end because the page comes from wordpress.