-
Notifications
You must be signed in to change notification settings - Fork 2
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
25. Make app more responsive for mobile compatibility #58
Conversation
Visit the preview URL for this PR (updated for commit 7d575f5): https://tcl-66-smart-shopping-list--pr58-an-issue-25-fx8kcv6k.web.app (expires Thu, 11 Apr 2024 03:54:06 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 2dc16cb2a79cbd6723fdc511b73e0743df1d18d0 |
src/views/List.jsx
Outdated
<img | ||
src={pageTitle} | ||
alt="The Collab Lab" | ||
className="mx-auto xsm:h-[24px] w-auto sm:hidden" |
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.
In the list view, the logo covers the share list button and the avatar count in the iPhoneSE view. I have some proposed code below that corrects this problem which you can copy and paste if you want.
Current iPhoneSE view
Using My Below Code
className="flex justify-start ml-6 xsm:h-[24px] w-auto sm:hidden"
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.
@eonflower, Awesome job on the logo and the responsiveness. I love how the app looks, especially in mobile view! Great job on making the code in the Navigation Bar component more DRY.
There's one change to make for the List view and then you can merge. I left a comment about it containing screenshots and included a potential solution that you can copy and paste if you wish.
Got it! I fixed it @3campos :) |
@eonflower Just to confirm what I'm seeing, the logo has been removed from the top of the list view. If that was intentional, then you're good to merge. |
@3campos yeah I removed it from the main page since it's in the nav bar. I felt like it wasn't necessary to have it in both spots. |
Description
Related Issue
closes #55
Acceptance Criteria
Type of Changes
enhancement, accessibility
Updates
Before
Screen.Recording.2024-04-02.at.10.01.13.PM.mov
After
Untitled.mov
Testing Steps / QA Criteria