-
-
Notifications
You must be signed in to change notification settings - Fork 11.3k
Fix: Mobile and Tablet UI issues on /members
#24503
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?
Fix: Mobile and Tablet UI issues on /members
#24503
Conversation
WalkthroughAdds Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
ghost/admin/app/styles/layouts/members.css (2)
8-10: Restrict overflow rule to smaller breakpoints (and fix minor formatting).
.view-containeris a high-level utility that’s reused across many admin screens. Adding a globaloverflow-x: hidden;risks masking legitimate horizontal scroll on wider layouts (e.g. long tables in other views). Consider moving this rule inside a breakpoint or a route-specific wrapper instead of the top-level container, and clean up the extra space after the selector.- .view-container { - overflow-x: hidden; - } +@media (max-width: 1100px) { /* or a route-specific selector */ + .members-view .view-container { + overflow-x: hidden; + } +}
326-333: Ensure truncated content remains readable on ≤ 440 px screens.The fixed 160 px width solves the layout crush, but without an explicit
text-overflow: ellipsistitles/emails may simply disappear beyond the cell edge (now hidden). Consider adding the common truncation trio to provide a visual cue:width: 160px; min-width: 160px; max-width: 160px; - overflow-x: hidden; + overflow: hidden; + white-space: nowrap; + text-overflow: ellipsis;Also note that the earlier default rule (30% / min-width 360 px) still applies above 440 px; confirm the breakpoint handoff doesn’t introduce a sudden jump around 441 px.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ghost/admin/app/styles/layouts/members.css(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: kevinansfield
PR: TryGhost/Ghost#23824
File: ghost/core/core/server/services/email-service/email-templates/partials/styles.hbs:919-926
Timestamp: 2025-06-13T11:57:58.226Z
Learning: In `ghost/core/core/server/services/email-service/email-templates/partials/styles.hbs`, some style blocks (e.g., `.latest-post p` and `.latest-post p a`) still use the legacy colour `#73818c` on purpose; they are later overridden by `emailCustomization` feature rules, as noted by inline TODO comments. These occurrences should not be flagged as inconsistencies.
ghost/admin/app/styles/layouts/members.css (1)
Learnt from: kevinansfield
PR: #23824
File: ghost/core/core/server/services/email-service/email-templates/partials/styles.hbs:919-926
Timestamp: 2025-06-13T11:57:58.226Z
Learning: In ghost/core/core/server/services/email-service/email-templates/partials/styles.hbs, some style blocks (e.g., .latest-post p and .latest-post p a) still use the legacy colour #73818c on purpose; they are later overridden by emailCustomization feature rules, as noted by inline TODO comments. These occurrences should not be flagged as inconsistencies.
ref TryGhost#24501 On mobile resolutions, this fixes an issues that causes the member details column to take most of the available viewport width, which caused readability issues with the rest of the table's columns. On tablet resolutions, this fixes an issue that caused the columns to visually overlap when scrolling horizontally.
This ensures that the overflow value is specific to tablet resolutions and restricted to the members section.
Removes a change which was inadvertently committed and no longer needed.
This change aligns with Tailwind, to identify scren resolutions upto 480px as mobile.
07d3ec0 to
2bb817c
Compare
|
Resolved merge conflicts and PR is ready for review. |
This PR resolves mobile and tablet UI bugs on the
/memberspage. More specifically, the following issues are fixed (tracked in #24501):Changes