Enhancement: Responsive Styles Update #6
Conversation
added a media query for tablets and phones removed some hardcoded styles and replaced them with more robust css in styles.css fixed invoice preview overflow & misc changes :3
|
@jacneeley is attempting to deploy a commit to the Eshita's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughCSS and HTML updates adjust theming and make the invoice UI responsive. home.css tweaks background colors and adds a mobile hero heading size. invoice.html restructures labels and wraps the download button. style.css adds extensive responsive rules, spacing, and alignment changes for invoice forms and previews. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–60 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
home.css (1)
30-30: Fix invalid CSS value with stray space:20 px→20pxThe space between the number and unit makes the declaration invalid; the margin-left is currently not applied.
Apply this diff:
- margin-left: 20 px; + margin-left: 20px;style.css (1)
118-118: Fix invalid color values missing “#” (currently not applied)These declarations are missing the leading “#”, so they’re ignored by the browser.
Apply this diff:
- background-color: ffffff; + background-color: #ffffff; @@ - background-color: ffffff; + background-color: #ffffff; @@ - background-color: ffffff; + background-color: #ffffff;Also applies to: 145-145, 149-149
🧹 Nitpick comments (7)
home.css (1)
97-101: Consider fluid type for mobile hero heading4rem may still be large on very small screens. A clamp() can scale more smoothly across devices.
Apply this diff:
- .hero h1{ - font-size: 4rem; - } + .hero h1{ + /* scales between 2.5rem and 4rem based on viewport width */ + font-size: clamp(2.5rem, 6vw, 4rem); + }style.css (4)
235-237: Leverage the new.download-pdf-btnwrapper for layoutRight now, only max-width is set; adding centering and alignment improves layout on wide screens and works well with the mobile centering you added to
#downloadPDF.Apply this diff:
-.download-pdf-btn{ - max-width: 1536px; -} +.download-pdf-btn{ + max-width: 1536px; + width: 100%; + margin: 0 auto; + padding: 0 20px; + display: flex; + justify-content: flex-end; +}Optionally, center on small screens by extending your media query:
@media (max-width: 1024px) { + .download-pdf-btn{ + justify-content: center; + } }
274-281: Avoid attribute selectors for performance and specificityUsing
[style*="text-align: right"]is brittle and slower. If feasible, prefer a semantic class on those cells (e.g.,.align-right) that you can override in the media query. If changing HTML isn’t in-scope, keep as-is.As a future improvement, consider:
/* HTML: add class="align-right" to those TDs */ .form-preview td.align-right { text-align: right; } @media (max-width: 1024px) { .form-preview td.align-right { text-align: left; } }
283-289: Add focus styles for keyboard accessibilityThe hover effect is nice; include a visible focus indicator for keyboard users.
Apply this diff:
#downloadPDF { display: block; margin-top: 20px; margin-left: auto; margin-right: .7rem; margin-bottom: 20px; padding: 10px 20px; font-size: 14px; background-color: #A78BFA; color: white; border: none; border-radius: 6px; cursor: pointer; transition: transform 0.3s ease, background-color 0.3s ease; } + +#downloadPDF:focus-visible { + outline: 3px solid #193497; + outline-offset: 2px; +}
334-445: Responsive reflow is comprehensive; add two small safety tweaksOverall, the stacked layout, padding, and scroll handling address the overflow issues well. Two suggestions to harden small-screen behavior:
- Override the global min-width on right-aligned cells in the mobile block to avoid any residual overflow on very narrow devices.
- Ensure images shrink to fit width without distortion.
Apply this diff inside the media query:
.form-preview td[style*="text-align: right"] { - text-align: left !important; + text-align: left !important; + min-width: 0; /* avoid unintended horizontal overflow on very narrow screens */ } .form-preview img { - height: 50px !important; + height: 50px !important; + max-width: 100%; + object-fit: contain; margin-bottom: 10px; }invoice.html (2)
376-378: Wrap for Download PDF is correct; pair with CSS to alignThe new
.download-pdf-btnwrapper is a good hook. Ensure the corresponding CSS centers on mobile and aligns right on wider screens (see suggested style.css diff).If desired, also add
type="button"to be defensive in case this button is ever moved into a form:- <button id="downloadPDF">Download PDF</button> + <button id="downloadPDF" type="button">Download PDF</button>
279-283: Consider removing inline image height and use CSS class insteadThis avoids the need for
!importantin CSS and keeps presentation out of markup.Example:
- HTML:
- <img src="gilogo.png" alt="GENINVOICE Logo" style="height: 60px" /> + <img src="gilogo.png" alt="GENINVOICE Logo" class="invoice-logo" />
- CSS (outside media, and inside media query):
.invoice-logo { height: 60px; } @media (max-width: 1024px) { .invoice-logo { height: 50px; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
home.css(3 hunks)invoice.html(2 hunks)style.css(8 hunks)
🔇 Additional comments (6)
home.css (2)
5-5: Theme background color update looks goodThe body background update aligns with the new visual direction and complements the invoice page styles.
16-16: Navbar background color aligned with bodyConsistent with the updated theme; no issues here.
style.css (3)
106-113: Textarea box-sizing + full width is a solid baseGood call ensuring consistent sizing and preventing manual resize in the form context.
179-190: Add-row button spacing improvementThe added bottom margin improves visual rhythm between the add-row control and subsequent content. Looks good.
267-273: Invoice number/date styling is appropriateThe padding and font-size give these fields consistent presence without inline styles. Good move.
invoice.html (1)
286-297: Inline font-size removals and invoice-number break are goodMoving presentation to CSS and inserting a conditional line break improves responsiveness and maintainability. No issues found.
jacneeley
changed the title
1 participant
Issue #2 Overview:
Detailed
Accessibility Stuff
I changed the background color slightly because the original colors were very similar and it was hurting my eyes ngl. Just let me know if you want it reverted back to the original.
I tested the following devices using firefox responsive tools:
iphone 12/13 mini is really great for testing because they are very small.
Fixing Overflows
The main changes were fixing the overflows with all of the tables. Tables are notoriously painful to make responsive because they don't really respond well to width styling and what not. Their width is primarily determined by the column widths, so the best course of action is to find a responsive way to style
th's,tr's andtd's. A lot of devs will cheat and build tables using grid-layouts and flex-boxes. I didn't want to do that and change a ton of html. I feel my solution is a good compromise. Check out this StackOverflow thread for more on this.Media Queries
I opted to just create one media query for mobile and tablets. There was a lot of ugly wrapping when I tried to write styles that would maintain the flex row direction of the forms for tablets. I felt it was simpler to just keep the
flex-directionascolumnif the width of the screen is <= 1024. Anything larger than that is entering desktop territory or 16:9 width. I just could not get it to look the way I wanted when testing for iPad Pro. Maybe somebody better than me could tackle that. Idk I am also lazy and feel its easier to maintain if tablets follow the same rules as phones.Responsive Samples
Mobile
Tablet (iPad)
Summary by CodeRabbit
New Features
Style