Improve styling to be more responsive/mobile friendly#63
Improve styling to be more responsive/mobile friendly#63nooreldeensalah wants to merge 6 commits intomainfrom
Conversation
nooreldeensalah
temporarily deployed
to
preview
GitHub Actions
Inactive
WalkthroughResponsive and mobile-focused UI refinements across pages and components, mobile-specific CSS and viewport metadata added, navbar gains a client-side mobile menu, narrator grid virtualization made responsive by computing itemsPer-row and card height dynamically. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Navbar
participant Router
User->>Navbar: Tap Menu button
alt open menu
Navbar->>Navbar: set isMobileMenuOpen = true
Navbar-->>User: Render mobile nav panel
else close menu
Navbar->>Navbar: set isMobileMenuOpen = false
end
User->>Navbar: Tap nav link
Navbar->>Navbar: set isMobileMenuOpen = false
Navbar->>Router: Navigate to route
Router-->>User: New page renders
sequenceDiagram
participant Window
participant useIsMobile
participant NarratorGrid
participant Virtualizer
Window-->>useIsMobile: initial/resize width
useIsMobile-->>NarratorGrid: isMobile / breakpoint info
NarratorGrid->>NarratorGrid: compute itemsPerRow & cardHeight (useEffect)
NarratorGrid->>Virtualizer: update sizeEstimate & rowGenerator
Virtualizer-->>NarratorGrid: virtual rows
NarratorGrid-->>Window: render responsive virtualized grid
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
components/navbar.tsx (1)
62-72: Consider adding keyboard navigation support for the mobile menu.The mobile menu button works well with mouse/touch, but consider enhancing keyboard accessibility:
- Handle
Escapekey to close the menu- Trap focus within the mobile menu when open
- Consider
aria-expandedattribute on the button<button onClick={() => setIsMobileMenuOpen(!isMobileMenuOpen)} + onKeyDown={(e) => { + if (e.key === 'Escape') setIsMobileMenuOpen(false); + }} className="p-2 transition-colors hover:bg-parchment md:hidden" aria-label="Toggle menu" + aria-expanded={isMobileMenuOpen} >components/narrator-grid.tsx (1)
122-138: Consider adding window resize listener for dynamic viewport changes.The useEffect only runs when
isMobilechanges, which typically happens once during mount. If a user resizes their browser window (e.g., from mobile to desktop viewport), the grid won't adapt to the new width.Add a resize listener to handle viewport changes:
useEffect(() => { if (isMobile === undefined) return; - if (isMobile) { + const updateLayout = () => { + if (isMobile) { - setItemsPerRow(1); - setCardHeight(280); - } else if (window.innerWidth < 1024) { - setItemsPerRow(2); - setCardHeight(300); - } else if (window.innerWidth < 1280) { - setItemsPerRow(3); - setCardHeight(320); - } else { - setItemsPerRow(4); - setCardHeight(320); - } + setItemsPerRow(1); + setCardHeight(280); + } else if (window.innerWidth < 1024) { + setItemsPerRow(2); + setCardHeight(300); + } else if (window.innerWidth < 1280) { + setItemsPerRow(3); + setCardHeight(320); + } else { + setItemsPerRow(4); + setCardHeight(320); + } + }; + + updateLayout(); + window.addEventListener('resize', updateLayout); + return () => window.removeEventListener('resize', updateLayout); }, [isMobile]);Note: Consider debouncing the resize handler to improve performance.
app/hadith/[source]/[chapter]/[hadithNo]/page.tsx (1)
127-127: Fixed height on mobile may truncate visualization.The network visualization section uses
h-[500px]on mobile. Depending on the complexity of the transmission chain, this fixed height might not be sufficient for all hadiths, potentially requiring excessive scrolling within the component.Consider using
min-h-[500px]instead ofh-[500px]to allow the visualization to expand if needed:- <section className="relative h-[500px] border-4 border-black bg-white p-1 shadow-[4px_4px_0px_0px_rgba(0,0,0,1)] md:h-[600px] md:shadow-[8px_8px_0px_0px_rgba(0,0,0,1)] lg:col-span-9 lg:h-screen"> + <section className="relative min-h-[500px] border-4 border-black bg-white p-1 shadow-[4px_4px_0px_0px_rgba(0,0,0,1)] md:min-h-[600px] md:shadow-[8px_8px_0px_0px_rgba(0,0,0,1)] lg:col-span-9 lg:h-screen">This would provide better flexibility while maintaining the minimum height constraint.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
app/about/page.tsx(1 hunks)app/globals.css(1 hunks)app/hadith/[source]/[chapter]/[hadithNo]/page.tsx(1 hunks)app/layout.tsx(1 hunks)app/narrator/[name]/page.tsx(4 hunks)app/narrator/page.tsx(1 hunks)app/search/page.tsx(4 hunks)components/chapter-page.tsx(1 hunks)components/footer.tsx(1 hunks)components/hadith-card.tsx(3 hunks)components/hadith-explanation-card.tsx(1 hunks)components/hadith-list.tsx(2 hunks)components/hadith-text-card.tsx(1 hunks)components/narrator-grid.tsx(9 hunks)components/navbar.tsx(2 hunks)lib/layout-constants.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
app/search/page.tsx (1)
lib/arabic-utils.ts (1)
arabicTexts(2-57)
components/hadith-list.tsx (1)
lib/layout-constants.ts (1)
LAYOUT(2-24)
components/hadith-text-card.tsx (1)
components/ui/card.tsx (1)
Card(79-79)
app/narrator/[name]/page.tsx (2)
components/brutalist-card.tsx (1)
BrutalistCard(6-17)lib/grade-mapping.ts (2)
getBlessings(51-64)getArabicGrade(47-49)
components/chapter-page.tsx (4)
lib/layout-constants.ts (1)
LAYOUT(2-24)lib/arabic-utils.ts (2)
arabicTexts(2-57)formatArabicCount(73-82)lib/utils.ts (1)
cleanName(8-17)lib/book-mapping.ts (1)
getArabicSource(10-12)
components/hadith-card.tsx (1)
lib/arabic-utils.ts (2)
toArabicNumerals(60-65)arabicTexts(2-57)
⏰ 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)
- GitHub Check: deploy
🔇 Additional comments (24)
components/footer.tsx (1)
6-6: LGTM! Mobile-first padding adjustment.The responsive vertical padding (py-4 on mobile, md:py-6 on desktop) aligns well with the mobile-first approach and maintains appropriate footer spacing across breakpoints.
app/about/page.tsx (1)
3-4: LGTM! Consistent mobile-first responsive adjustments.The container padding and heading typography changes follow the mobile-first pattern consistently:
- Reduced base padding (py-6) scales up on medium screens (md:py-8)
- Smaller base heading (text-2xl) scales up on medium screens (md:text-3xl)
These changes align well with the broader responsive design strategy.
app/layout.tsx (1)
20-24: LGTM! Essential viewport configuration for responsive design.The viewport metadata is properly configured:
width: "device-width"enables responsive layoutsinitialScale: 1prevents unintended zoomingmaximumScale: 5permits user zoom for accessibilityThis is a foundational change that enables the responsive adjustments throughout the PR.
app/globals.css (1)
50-54: LGTM! Prevents horizontal scroll on mobile.The
overflow-x: hiddenrule onhtmlandbodyprevents horizontal scrolling issues that commonly occur on mobile devices when content or elements exceed viewport width.components/hadith-explanation-card.tsx (1)
9-13: LGTM! Comprehensive responsive adjustments.The card, heading, and prose container all follow the mobile-first pattern consistently:
- Reduced base padding (p-4) and shadow scale up on medium screens
- Heading typography (text-xl) and spacing (mb-3) scale up appropriately
- Prose size (prose-sm) scales to prose-lg on medium screens
These changes improve mobile readability while preserving desktop presentation.
components/navbar.tsx (2)
1-9: LGTM! Proper client component setup for mobile menu state.The component correctly uses
"use client"directive anduseStateto manage mobile menu state. The icon imports fromlucide-reactare appropriate for the menu toggle functionality.
77-106: Verify mobile menu closes on viewport resize.The mobile menu state persists across viewport changes. If a user opens the mobile menu, then resizes to desktop width, the menu state remains
true(though hidden by CSS). While not critical, consider adding a resize listener to close the menu when crossing themdbreakpoint.Test scenario: Open mobile menu → resize browser to desktop width → resize back to mobile. Verify the menu state is appropriate and doesn't cause unexpected behavior.
lib/layout-constants.ts (2)
13-23: LGTM! Well-structured responsive layout constants.The new responsive padding and gap utilities provide a consistent, reusable set of spacing values that align with the mobile-first approach. The naming convention is clear and the breakpoint usage (md:) is consistent throughout.
14-14: Verify small-screen padding adjustments across all CONTAINER_CLASSES consumers
CONTAINER_CLASSES changed frompy-8topy-4 md:py-8. Confirm the reduced vertical padding on small viewports is acceptable in:
- components/hadith-list.tsx:16
- components/chapter-page.tsx:37
- components/chapter-list.tsx:24
components/hadith-list.tsx (2)
16-16: LGTM! Proper usage of responsive layout constants.The component correctly combines
LAYOUT.CONTAINER_CLASSES(which provides responsive vertical padding) with additional responsive horizontal padding (px-2 md:px-4), following the established mobile-first pattern.
32-32: Verify that pl-1 (4px) provides sufficient spacing on mobile.The left padding was reduced from
pl-4(16px) topl-1(4px) on mobile devices. This minimal padding might make the layout feel cramped, especially considering the virtualized list already has limited horizontal space on small screens.Please test the hadith list on mobile devices to ensure:
- Cards don't appear too close to the container edge
- Touch interaction with cards is comfortable
- Visual hierarchy is maintained
Consider using
pl-2(8px) as a middle ground ifpl-1feels too tight.components/narrator-grid.tsx (4)
75-77: Good progressive enhancement with responsive filter text.The approach of showing "الكل" on mobile and "جميع الدرجات" on larger screens is appropriate for space-constrained layouts.
185-185: Correct virtualization size calculation.The
estimateSizenow uses the dynamiccardHeightinstead of a fixed constant, which is essential for accurate virtualization across different viewport sizes.
214-214: Responsive scroll container height is appropriate.The container height scales from 600px on mobile to 800px on desktop, providing adequate viewing area for both contexts.
7-7: Ignore SSR hydration undefined‐return suggestion for useIsMobile
useIsMobile coerces its initialundefinedstate tofalsevia!!isMobile, so server and client markup will match—no additional SSR guard orundefinedreturn needed.Likely an incorrect or invalid review comment.
app/hadith/[source]/[chapter]/[hadithNo]/page.tsx (3)
120-120: Verify adequate horizontal padding on mobile.The container uses
px-2on mobile, which provides only 8px of padding. For touch-friendly interfaces, consider whether this provides sufficient buffer from screen edges.Test on actual mobile devices to ensure:
- Text and interactive elements don't feel cramped
- Tap targets aren't too close to screen edges
- Content remains comfortably readable
122-125: Mobile-first stacking is appropriate.The layout correctly stacks the hadith text and explanation vertically on mobile (grid-cols-1) before transitioning to the side-by-side layout on large screens (lg:col-span-3). This prioritizes content readability on small screens.
127-127: Appropriate shadow scaling for brutalist design.The shadow scales from 4px on mobile to 8px on medium screens, maintaining the brutalist aesthetic while reducing visual weight on smaller screens. This is a good responsive design choice.
app/narrator/[name]/page.tsx (6)
161-171: Responsive spacing and typography scaling is consistent.The InfoSection properly scales gaps (gap-4 → md:gap-6), decorative bar thickness (border-t-2 → md:border-t-4), and heading text (text-xl → md:text-2xl), maintaining visual hierarchy across breakpoints.
172-172: Dynamic grid columns provide good content adaptation.The info cards grid transitions from 1 column (mobile) to 2 columns (medium) to 3 columns (large), ensuring readability at all viewport sizes while maximizing space usage on larger screens.
178-181: Creative use of decorative underline with responsive sizing.The technique of using an absolutely positioned background element that scales from h-2 to h-3 across breakpoints is a nice touch that maintains the brutalist aesthetic. The translate adjustment keeps it properly positioned.
230-240: Well-structured responsive grid layout.The main content area effectively uses a 1-column mobile layout that expands to a 12-column grid on large screens, with the sidebar taking 3 columns and the main content taking 9. The explicit
lg:order-*classes ensure proper visual ordering.
241-241: Min-height ensures adequate visualization space.Using
min-h-[400px]on mobile andmd:min-h-screenon larger screens provides flexibility for the transmission chain visualization while ensuring a minimum usable height. This is more appropriate than a fixed height.
183-183: Verify text overflow behavior in hover state.The info card content uses
max-h-[30vh] overflow-y-autowithin the expanded hover state. On mobile devices with limited viewport height, 30vh might be quite small. Ensure scrolling works smoothly on touch devices.Test the hover/expansion behavior on mobile:
- Does the expansion work on touch devices (typically requires tap, not hover)?
- Is 30vh sufficient for readable content on small phones?
- Does the overflow scrolling feel natural with touch gestures?
| <FaGithub className="text-base md:text-lg" /> | ||
| <span className="relative z-10 text-xs md:text-sm">الكود</span> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify text legibility at text-xs on small screens.
The text size is reduced to text-xs (12px) on mobile devices. While this saves space, please confirm that the footer label text remains legible at this size, especially for users with visual impairments.
🌐 Web query:
What is the minimum recommended font size for mobile text according to WCAG and mobile accessibility guidelines?
💡 Result:
Short answer:
- WCAG does not mandate a single minimum mobile font-size, but defines "large scale (text)" as at least 18pt (or 14pt bold) when delivered and requires text to be resizable (e.g., 200%). [1]
- Practical mobile guidance: many platform and accessibility guides recommend ~16px (≈12pt) or larger for body text as a minimum for readability on mobile. Also follow platform Dynamic Type / scalable units (iOS Dynamic Type, Android sp). [2][3]
Sources:
[1] WCAG 2.1 (large text / resize text).
[2] Apple — Larger Text / Dynamic Type guidance.
[3] Web accessibility guidance recommending ~16px baseline and use of relative units.
Ensure footer label meets mobile readability guidelines
text-xs (12px) on small screens falls below the ~16px baseline recommended for mobile; either bump it to at least text-sm (14px) or confirm 12px renders legibly and supports 200% resizing per WCAG.
🤖 Prompt for AI Agents
In components/footer.tsx around lines 14 to 15, the span uses "text-xs
md:text-sm" which sets ~12px on small screens; update the class to use at least
"text-sm" for small screens (e.g., "relative z-10 text-sm md:text-sm") so mobile
font meets the recommended baseline and still switches to the md size on larger
viewports.
| const [itemsPerRow, setItemsPerRow] = useState(4); | ||
| const [cardHeight, setCardHeight] = useState(320); | ||
|
|
||
| useEffect(() => { | ||
| if (isMobile === undefined) return; | ||
|
|
||
| if (isMobile) { | ||
| setItemsPerRow(1); | ||
| setCardHeight(280); | ||
| } else if (window.innerWidth < 1024) { | ||
| setItemsPerRow(2); | ||
| setCardHeight(300); | ||
| } else if (window.innerWidth < 1280) { | ||
| setItemsPerRow(3); | ||
| setCardHeight(320); | ||
| } else { | ||
| setItemsPerRow(4); | ||
| setCardHeight(320); | ||
| } | ||
| }, [isMobile]); |
There was a problem hiding this comment.
Potential layout shift on initial render.
The component initializes itemsPerRow to 4 and cardHeight to 320 (desktop values), then updates them in a useEffect based on isMobile. This will cause a visible layout shift on mobile devices after the first render, as the grid will initially render with 4 columns before adjusting to 1 column.
Consider initializing with mobile-first values to minimize layout shift:
- const [itemsPerRow, setItemsPerRow] = useState(4);
- const [cardHeight, setCardHeight] = useState(320);
+ const [itemsPerRow, setItemsPerRow] = useState(1);
+ const [cardHeight, setCardHeight] = useState(280);This aligns with the mobile-first approach used elsewhere in the PR.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [itemsPerRow, setItemsPerRow] = useState(4); | |
| const [cardHeight, setCardHeight] = useState(320); | |
| useEffect(() => { | |
| if (isMobile === undefined) return; | |
| if (isMobile) { | |
| setItemsPerRow(1); | |
| setCardHeight(280); | |
| } else if (window.innerWidth < 1024) { | |
| setItemsPerRow(2); | |
| setCardHeight(300); | |
| } else if (window.innerWidth < 1280) { | |
| setItemsPerRow(3); | |
| setCardHeight(320); | |
| } else { | |
| setItemsPerRow(4); | |
| setCardHeight(320); | |
| } | |
| }, [isMobile]); | |
| // Initialize mobile-first to avoid layout shift on initial render | |
| - const [itemsPerRow, setItemsPerRow] = useState(4); | |
| const [itemsPerRow, setItemsPerRow] = useState(1); | |
| const [cardHeight, setCardHeight] = useState(280); | |
| useEffect(() => { | |
| if (isMobile === undefined) return; | |
| if (isMobile) { | |
| setItemsPerRow(1); | |
| setCardHeight(280); | |
| } else if (window.innerWidth < 1024) { | |
| setItemsPerRow(2); | |
| setCardHeight(300); | |
| } else if (window.innerWidth < 1280) { | |
| setItemsPerRow(3); | |
| setCardHeight(320); | |
| } else { | |
| setItemsPerRow(4); | |
| setCardHeight(320); | |
| } | |
| }, [isMobile]); |
🤖 Prompt for AI Agents
In components/narrator-grid.tsx around lines 119 to 138, the state defaults are
set to desktop values which cause a layout shift on initial render on mobile;
change the initial state to mobile-first values (e.g., itemsPerRow=1,
cardHeight=280) or compute initial values from isMobile/window in the useState
initializer so the first render matches the device size, keep the existing
useEffect to adjust for other breakpoints.
85631a1 to
fc8ded9
Compare
nooreldeensalah
temporarily deployed
to
preview
GitHub Actions
Inactive
nooreldeensalah
temporarily deployed
to
preview
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
components/narrator-grid.tsx (1)
119-138: Layout shift issue previously flagged.The desktop-first initial state values (
itemsPerRow: 4,cardHeight: 320) will cause a visible layout shift on mobile devices during the first render, as noted in the previous review. Consider initializing with mobile-first values or computing initial values based onisMobileto minimize this shift.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/layout.tsx(3 hunks)components/chapter-page.tsx(2 hunks)components/footer.tsx(1 hunks)components/hadith-card.tsx(3 hunks)components/hadith-list.tsx(2 hunks)components/narrator-card.tsx(2 hunks)components/narrator-grid.tsx(11 hunks)lib/layout-constants.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- components/hadith-list.tsx
- components/footer.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
components/chapter-page.tsx (4)
lib/layout-constants.ts (1)
LAYOUT(2-24)lib/arabic-utils.ts (2)
arabicTexts(2-57)formatArabicCount(73-82)lib/utils.ts (1)
cleanName(8-17)lib/book-mapping.ts (1)
getArabicSource(10-12)
components/hadith-card.tsx (1)
lib/arabic-utils.ts (2)
toArabicNumerals(60-65)arabicTexts(2-57)
⏰ 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)
- GitHub Check: deploy
🔇 Additional comments (11)
app/layout.tsx (2)
13-17: LGTM! Proper viewport configuration for mobile rendering.The viewport export follows Next.js 15.x metadata conventions and includes appropriate settings for mobile devices. The
maximumScale: 5value correctly allows users to zoom, meeting accessibility requirements.
63-65: LGTM! Responsive layout improvements.The addition of
relativepositioning enables proper layering of child components, and the responsive bottom padding (pb-20 md:pb-24) follows the mobile-first approach consistently used throughout this PR.lib/layout-constants.ts (2)
14-23: LGTM! Well-structured responsive utilities.The new responsive padding and gap utilities follow Tailwind's mobile-first breakpoint pattern and provide consistent spacing scales across the application.
5-5: Type change is safe; no numeric usage detected.Verified that VIRTUALIZED_CONTAINER_HEIGHT is only used in React style props—which accept string values for CSS—and isn’t involved in any numeric operations.
components/narrator-grid.tsx (2)
48-101: LGTM! Comprehensive responsive refinements.The filter panel updates implement thorough responsive adjustments including:
- Scaled input and button text sizes
- Responsive icon sizing
- Shortened filter labels on mobile (e.g., "الكل" vs "جميع الدرجات")
- Appropriate spacing and margins across breakpoints
These changes enhance usability on smaller screens.
214-246: LGTM! Dynamic virtualization with responsive grid.The virtualization implementation correctly:
- Uses
measureElementanddata-indexfor accurate sizing- Applies responsive grid classes (
grid-cols-1 sm:grid-cols-2 lg:grid-cols-3 xl:grid-cols-4)- Includes dynamic
paddingBottombased on GAPThis enables smooth scrolling with proper measurement across all viewport sizes.
components/chapter-page.tsx (3)
36-38: LGTM! Proper use of layout constants.The container correctly combines
LAYOUT.CONTAINER_CLASSESwithLAYOUT.PADDING_X_RESPONSIVEto apply consistent responsive padding across the page.
51-93: LGTM! Thorough responsive header implementation.The chapter header includes comprehensive responsive adjustments:
- Scaled padding and spacing (
p-4 md:p-8,gap-2 md:gap-4)- Typography scaling (
text-xl md:text-3xl,text-base md:text-lg)- Responsive icon and badge sizing
- Lighter borders and shadows on mobile (
border-2vsmd:border-4)These changes maintain visual hierarchy while optimizing for touch interaction on smaller screens.
96-99: LGTM! Dynamic container height for virtualization.The hadith list container correctly uses the updated
VIRTUALIZED_CONTAINER_HEIGHTconstant (now a CSS calc() expression) to provide viewport-relative sizing that adapts to available screen space.components/hadith-card.tsx (2)
28-41: LGTM! Excellent mobile-first decorative element handling.The implementation properly handles decorative elements across breakpoints:
- Decorative corner hidden on very small screens (
hidden sm:block) to reduce clutter- Separate mobile ID badge (
sm:hidden) ensures the hadith ID remains visible and accessible on small screens without the decorative corner- Responsive sizing of decorative elements (
h-16 w-16 md:h-24 md:w-24)This approach prioritizes content readability on mobile while maintaining visual appeal on larger screens.
35-69: LGTM! Comprehensive responsive layout refinements.The card content applies thorough responsive adjustments:
- Progressive padding (
p-4 sm:p-6 sm:pl-12 sm:pt-8 md:pl-16 md:pt-10)- Scaled typography (
text-lg md:text-xl,text-xs md:text-sm,text-sm md:text-base)- Responsive metadata badges with proper touch targets
- Flexible wrapping for metadata row
These changes ensure readability and usability across all viewport sizes.
| <div className="absolute bottom-1 left-1"> | ||
| <Button | ||
| asChild | ||
| className="flex h-4 items-center gap-0.5 rounded-none border border-isnad-primary bg-isnad-primary px-1 text-[6px] font-bold text-isnad-background shadow-[1px_1px_0px_0px_theme(colors.isnad.primary)] transition-colors hover:bg-isnad-primary-hover focus-visible:ring-isnad-primary focus-visible:ring-offset-0" | ||
| > | ||
| <Link href={`/narrator/${name}`}> | ||
| <span className="leading-none">المزيد</span> | ||
| <ArrowUpRightIcon className="h-2 w-2" /> | ||
| </Link> | ||
| </Button> | ||
| </div> |
There was a problem hiding this comment.
Touch target size below accessibility guideline.
The button height is set to h-4 (16px), which falls short of the 44×44px minimum touch target mentioned in the PR objectives. This could impact mobile usability, especially since the "المزيد" (More) button appears to be the primary navigation element for this card.
Consider increasing the button size or adding adequate padding to meet the minimum touch target size:
<Button
asChild
- className="flex h-4 items-center gap-0.5 rounded-none border border-isnad-primary bg-isnad-primary px-1 text-[6px] font-bold text-isnad-background shadow-[1px_1px_0px_0px_theme(colors.isnad.primary)] transition-colors hover:bg-isnad-primary-hover focus-visible:ring-isnad-primary focus-visible:ring-offset-0"
+ className="flex min-h-[44px] min-w-[44px] items-center justify-center gap-0.5 rounded-none border border-isnad-primary bg-isnad-primary px-2 text-[6px] font-bold text-isnad-background shadow-[1px_1px_0px_0px_theme(colors.isnad.primary)] transition-colors hover:bg-isnad-primary-hover focus-visible:ring-isnad-primary focus-visible:ring-offset-0"
>Alternatively, if the small size is intentional for desktop and you're relying on responsive classes elsewhere, consider adding a responsive variant: h-4 md:min-h-[44px] md:min-w-[44px].
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className="absolute bottom-1 left-1"> | |
| <Button | |
| asChild | |
| className="flex h-4 items-center gap-0.5 rounded-none border border-isnad-primary bg-isnad-primary px-1 text-[6px] font-bold text-isnad-background shadow-[1px_1px_0px_0px_theme(colors.isnad.primary)] transition-colors hover:bg-isnad-primary-hover focus-visible:ring-isnad-primary focus-visible:ring-offset-0" | |
| > | |
| <Link href={`/narrator/${name}`}> | |
| <span className="leading-none">المزيد</span> | |
| <ArrowUpRightIcon className="h-2 w-2" /> | |
| </Link> | |
| </Button> | |
| </div> | |
| <div className="absolute bottom-1 left-1"> | |
| <Button | |
| asChild | |
| className="flex min-h-[44px] min-w-[44px] items-center justify-center gap-0.5 rounded-none border border-isnad-primary bg-isnad-primary px-2 text-[6px] font-bold text-isnad-background shadow-[1px_1px_0px_0px_theme(colors.isnad.primary)] transition-colors hover:bg-isnad-primary-hover focus-visible:ring-isnad-primary focus-visible:ring-offset-0" | |
| > | |
| <Link href={`/narrator/${name}`}> | |
| <span className="leading-none">المزيد</span> | |
| <ArrowUpRightIcon className="h-2 w-2" /> | |
| </Link> | |
| </Button> | |
| </div> |
1 participant
Overview
All pages and components have been updated to provide an optimal viewing experience across different screen sizes:
Changes by Component
1. Navigation (
components/navbar.tsx)✅ Changes:
2. Hadith Cards (
components/hadith-card.tsx)✅ Changes:
p-4 sm:p-6text-lg md:text-xlfor titles,text-sm md:text-basefor content3. Hadith List (
components/hadith-list.tsx)✅ Changes:
px-2 md:px-4pl-1 md:pl-44. Search Page (
app/search/page.tsx)✅ Changes:
px-2 md:px-4mx-2 md:mx-16p-3 md:p-45. Narrator Grid (
components/narrator-grid.tsx&app/narrator/page.tsx)✅ Changes:
useIsMobilehook for responsive behaviorh-[600px] md:h-[800px]text-2xl md:text-4xl6. Narrator Detail Page (
app/narrator/[name]/page.tsx)✅ Changes:
text-2xl md:text-4xlmin-h-[400px] md:min-h-screen7. Hadith Detail Page (
app/hadith/[source]/[chapter]/[hadithNo]/page.tsx)✅ Changes:
h-[500px] md:h-[600px] lg:h-screenpx-2 md:px-48. Hadith Text & Explanation Cards
✅ Changes:
p-4 md:p-6text-xl md:text-2xlprose-base md:prose-xlandprose-sm md:prose-lgshadow-[4px_4px...] md:shadow-[8px_8px...]9. Chapter Page (
components/chapter-page.tsx)✅ Changes:
h-6 w-6 md:h-8 md:w-8text-xl md:text-3xlborder-2 md:border-410. Footer (
components/footer.tsx)✅ Changes:
py-4 md:py-6text-base md:text-lgtext-xs md:text-sm11. About Page (
app/about/page.tsx)✅ Changes:
py-6 md:py-8text-2xl md:text-3xl12. Layout Constants (
lib/layout-constants.ts)✅ Changes:
CONTAINER_CLASSESwith responsive paddingPADDING_X_RESPONSIVE: "px-2 md:px-4"PADDING_Y_RESPONSIVE: "py-4 md:py-8"GAP_SMALL_RESPONSIVE,GAP_MEDIUM_RESPONSIVE,GAP_LARGE_RESPONSIVE13. Global Styles (
app/globals.css)✅ Changes:
14. Root Layout (
app/layout.tsx)✅ Changes:
Design Principles Applied
Breakpoints Used
Testing Recommendations
Future Improvements
Consider these additional enhancements:
Summary by CodeRabbit
New Features
Style
Bug Fixes