Finished mobile view debugging (except orange ticket issues) #499
Conversation
✅ Deploy Preview for dailp ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Is there a way you could deduplicate some of the styling in this file?
There was a problem hiding this comment.
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.
| onClick={(e) => { | ||
| // Open image in new tab for full view | ||
| window.open(e.currentTarget.src, "_blank") | ||
| }} |
There was a problem hiding this comment.
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.
I just added a title tag 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.
| width: "auto", | ||
| minWidth: "0px", | ||
| maxWidth: "200px", | ||
| } |
There was a problem hiding this comment.
You could change the fixed maxWidth to a variable or theme token so it’s easier to tweak later
There was a problem hiding this comment.
I changed this to half the width of the screen.
website/src/components/wordpress.tsx
Outdated
| style={{ | ||
| width: "100%", | ||
| maxWidth: "100%", | ||
| height: "auto", |
There was a problem hiding this comment.
I'm noticing that some styles are inline (like for <video> here) while others are in globalStyle. Would it make sense to consolidate for consistency and maintainability?
There was a problem hiding this comment.
I moved these into document.css.ts which is where the css for the wordpress.tsx file comes from. globalStyle isn't needed here because it's applied directly to the target element. globalStyle is useful because it allows you to target child elements you don't have direct access to.
| width: "auto !important", // Override inline width="300" | ||
| height: "auto !important", // Override inline height="300" | ||
| minWidth: "120px !important", | ||
| display: "inline-block !important", |
There was a problem hiding this comment.
The 120px minWidth could cause layout issues on small screens. Have we tested this on smaller devices?
There was a problem hiding this comment.
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.
Issue list is in this spreadsheet
https://docs.google.com/spreadsheets/d/1mxTOXmBQQTaa4usXe2uaPrO0-COXD1Gc9EcekPqwNqI/edit?gid=0#gid=0
Main Problems
Solution
!isDesktopdetectionclearandfloatto stack content more vertically rather than horizontallyconst parseOptionsin wordpress.tsx to improve readability with-webkit-optimize-contrast, click to enlarge images in case it's a really big table that you can't read properly, and responsive sizing on all platformsProblems/Limitations
dev.dailp.northeastern.edu/lessons/(page name)or a wordpress link likehttps://https//wp.dailp.northeastern.edu/lessons/(page name). I don't think the fix is in the codebase but rather in the Wordpress, just changing the redirects to/collections/cwkw/(page name)instead.