-
Notifications
You must be signed in to change notification settings - Fork 39
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
Update MessageBase to show date on new line for mobile screens #751
Conversation
Hi @humphd @tarasglek, Please review the PR and let me know if any changes are required. |
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.
Nice. A few things I notice:
Font Too Large, Vertical Gap Too Large
I think we can reduce the font size of the date, and amount of space between the two stacked divs:
Vertically Centre Star Icon
I wonder if we should vertically centre the star icon on mobile? It seems to be unaligned with anything now
Deploying chatcraft-org with Cloudflare Pages
|
Hi @humphd , |
color="gray.500" | ||
_dark={{ color: "gray.300" }} | ||
<Flex | ||
direction={{ base: "column", sm: "row" }} // Stack on mobile, row on larger screens |
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.
nit: prefer putting comments above vs. to the right of content, so lines don't extend horizontally. Vertical > horizontal.
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.
This is really nice, great work!
This Fixes #747
I added a Flex component around heading and date and set direction of flex to row for sm screens
Demo: