Koenig: Improve Unsplash Modal Responsiveness #1576
Conversation
The Unsplash modal is not optimised for mobile and tablet viewports and is practically unusable. These fixes ensure that the UX on mobile and tablet views works as one would expect it to.
WalkthroughImplements responsive column behavior and touch zoom guard in the Unsplash selector flow. Adds viewport-based column count calculation, a window resize listener to update masonry columns and reflow data, and forwards columnCount through UnsplashSearchModal to UnsplashSelector and UnsplashGallery. Introduces UnsplashService.getColumnCount accessor. Layout classes in UnsplashGallery, UnsplashSelector, and UnsplashImage are adjusted based on columnCount and viewport. Adds a mobile/touch early return to disable zoom on selection. Minor className tweaks in UnsplashZoomed. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
✨ 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 comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Zoom behaviour is handled on touch and non-touch devices. This also improves the CSS for better UX on small screens.
niranjan-uma-shankar
changed the title
…en width breakpoints. This ensures that the UI is overall aligned with column count behaviour
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
packages/kg-unsplash-selector/src/ui/UnsplashGallery.tsx (1)
102-109: ForwardcolumnCounttoGalleryLayoutin all branches
columnCountis only passed in the non-zoomed path. Zoomed and error states default to0, which changes padding unexpectedly. Forward it for consistency.Apply this diff:
// zoomed branch -<GalleryLayout - galleryRef={galleryRef} - zoomed={zoomed}> +<GalleryLayout + columnCount={columnCount} + galleryRef={galleryRef} + zoomed={zoomed}> // error branch -<GalleryLayout - galleryRef={galleryRef} - zoomed={zoomed}> +<GalleryLayout + columnCount={columnCount} + galleryRef={galleryRef} + zoomed={zoomed}>
🧹 Nitpick comments (4)
packages/kg-unsplash-selector/src/ui/UnsplashSelector.tsx (1)
26-32: Avoid conflicting padding classes in the headerWhen
columnCount < 3, bothpx-20andpx-5are applied; the latter wins but it's noisy. Compose mutually exclusive classes instead.-<header className={`flex shrink-0 items-center justify-between px-20 py-10 flex-row ${columnCount < 3 ? 'flex-col gap-3 px-5' : ''}`}> +<header className={`flex shrink-0 items-center justify-between py-10 ${columnCount < 3 ? 'flex-col gap-3 px-5' : 'flex-row px-20'}`}>packages/kg-unsplash-selector/src/ui/UnsplashImage.tsx (1)
64-68: Alt text: use author’s name for better a11yThe author avatar’s
alt="author"is generic. Prefer the actual name.-<img alt="author" className="mr-2 size-8 shrink-0 rounded-full" src={user.profile_image.medium} /> +<img alt={user.name} className="mr-2 size-8 shrink-0 rounded-full" src={user.profile_image.medium} />packages/kg-unsplash-selector/src/UnsplashSearchModal.tsx (2)
19-21: Breakpoints: consider centralizing to avoid driftThese width thresholds duplicate similar logic elsewhere (e.g., Tenor selector). Centralize in a shared constants file to keep UX consistent across selectors.
222-235: Pass a statefulcolumnCountinstead of calling the service in JSXRelying on
UnsplashLib.getColumnCount()inside JSX can cause an initial flicker (defaults to 3) before the resize effect runs. TrackcolumnCountin React state and update it inhandleResize; pass that down to children.Minimal changes (snippets show additions outside selected lines too):
- Add state near other state declarations:
const [columnCount, setColumnCount] = useState<number>(3);
- Update
handleResize:const newColumnCount = calculateColumnCountForViewport(); if (newColumnCount !== UnsplashLib.getColumnCount()) { masonryService.changeColumnCount(newColumnCount); setColumnCount(newColumnCount); // reflow + setDataset… }
- Pass state to children:
-<UnsplashSelector - closeModal={onClose} - columnCount={UnsplashLib.getColumnCount()} - handleSearch={handleSearch} -> +<UnsplashSelector + closeModal={onClose} + columnCount={columnCount} + handleSearch={handleSearch} +> … -<UnsplashGallery - columnCount={UnsplashLib.getColumnCount()} +<UnsplashGallery + columnCount={columnCount}This keeps header/search/gallery layout in sync without relying on a service read during render.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/kg-unsplash-selector/src/UnsplashSearchModal.tsx(4 hunks)packages/kg-unsplash-selector/src/api/UnsplashService.ts(1 hunks)packages/kg-unsplash-selector/src/ui/UnsplashGallery.tsx(4 hunks)packages/kg-unsplash-selector/src/ui/UnsplashImage.tsx(2 hunks)packages/kg-unsplash-selector/src/ui/UnsplashSelector.tsx(2 hunks)packages/kg-unsplash-selector/src/ui/UnsplashZoomed.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/kg-unsplash-selector/src/ui/UnsplashGallery.tsx (1)
packages/kg-unsplash-selector/src/UnsplashTypes.ts (1)
Photo(49-72)
packages/kg-unsplash-selector/src/UnsplashSearchModal.tsx (2)
packages/koenig-lexical/src/components/ui/TenorSelector.jsx (1)
TWO_COLUMN_WIDTH(8-8)packages/koenig-lexical/src/components/ui/cards/EmbedCard.jsx (1)
handleResize(44-80)
🔇 Additional comments (4)
packages/kg-unsplash-selector/src/ui/UnsplashSelector.tsx (1)
31-33: Search container responsiveness — LGTMConditional
max-w-smbased oncolumnCountcleanly makes the search bar full-width on narrow layouts.packages/kg-unsplash-selector/src/ui/UnsplashZoomed.tsx (1)
12-12: Usingsize-fullto fill the zoom container — LGTMThis ensures click-to-close works across the full overlay and keeps the zoomed image centered.
packages/kg-unsplash-selector/src/ui/UnsplashGallery.tsx (1)
83-91: Padding logic — LGTM (matches responsive intent)Inner scroll area uses
px-5for<3columns andpx-20otherwise; combined with outerpx-5for single-column this achieves the desired spacing across breakpoints.packages/kg-unsplash-selector/src/UnsplashSearchModal.tsx (1)
79-90: Viewport-to-columns helper — LGTMThe 1/2/3 column mapping is clear and matches the PR’s behavior spec.
| getColumnCount(): number { | ||
| return this.masonryService.columnCount; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Expose column count in the public interface to keep API consistent
getColumnCount() is public on the class but missing from IUnsplashService. Downstream code typed against the interface won't see it.
Apply this diff to the interface:
export interface IUnsplashService {
loadNew(): Promise<void>;
layoutPhotos(): void;
getColumns(): Photo[][] | [] | null;
+ getColumnCount(): number;
updateSearch(term: string): Promise<void>;
loadNextPage(): Promise<void>;
clearPhotos(): void;
triggerDownload(photo: Photo): void;
photos: Photo[];
searchIsRunning(): boolean;
}📝 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.
| getColumnCount(): number { | |
| return this.masonryService.columnCount; | |
| } | |
| export interface IUnsplashService { | |
| loadNew(): Promise<void>; | |
| layoutPhotos(): void; | |
| getColumns(): Photo[][] | [] | null; | |
| getColumnCount(): number; | |
| updateSearch(term: string): Promise<void>; | |
| loadNextPage(): Promise<void>; | |
| clearPhotos(): void; | |
| triggerDownload(photo: Photo): void; | |
| photos: Photo[]; | |
| searchIsRunning(): boolean; | |
| } |
🤖 Prompt for AI Agents
In packages/kg-unsplash-selector/src/api/UnsplashService.ts around lines 48 to
50, the class exposes a public getColumnCount(): number method but the
IUnsplashService interface does not declare it; add getColumnCount(): number to
the IUnsplashService interface (exported where it is defined) so downstream code
typed against the interface will see the method and keep the public API
consistent.
| return ( | ||
| <div | ||
| className={`relative mb-6 block ${zoomed ? 'h-full w-[max-content] cursor-zoom-out' : 'w-full cursor-zoom-in'}`} | ||
| className={`relative mb-6 block ${zoomed ? 'h-full w-[max-content] cursor-zoom-out' : 'w-full [@media(min-width:540px)]:cursor-zoom-in'}`} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve touch UX: make overlay reliably reveal on tap (not just CSS :hover)
Relying on :hover alone is inconsistent on touch. Add focus/active triggers and make the container focusable so a tap reliably reveals controls as per the PR goal.
Within the container div, add tabIndex:
-<div
+<div
+ tabIndex={0}
className={`relative mb-6 block ${zoomed ? 'h-full w-[max-content] cursor-zoom-out' : 'w-full [@media(min-width:540px)]:cursor-zoom-in'}`}Then update the overlay to also show on focus/active (outside the selected range, shown for completeness):
-<div className="absolute inset-0 flex flex-col justify-between bg-gradient-to-b from-black/5 via-black/5 to-black/30 p-5 opacity-0 transition-all ease-in-out hover:opacity-100">
+<div className="absolute inset-0 flex flex-col justify-between bg-gradient-to-b from-black/5 via-black/5 to-black/30 p-5 opacity-0 transition-all ease-in-out hover:opacity-100 focus-within:opacity-100 active:opacity-100">This preserves hover on desktop and enables single-tap reveal on touch.
🤖 Prompt for AI Agents
In packages/kg-unsplash-selector/src/ui/UnsplashImage.tsx around line 35, the
container currently only reveals the overlay via CSS :hover which is unreliable
on touch; make the container focusable by adding a tabIndex (e.g., tabIndex={0})
to the wrapping div and update the overlay's utility classes so it becomes
visible not just on :hover but also on :focus and :active (e.g., add focus: and
active: variants to the same classes that show on hover) so a single tap or
keyboard focus reliably reveals controls while preserving desktop hover
behavior.
| React.useEffect(() => { | ||
| const handleResize = () => { | ||
| const newColumnCount = calculateColumnCountForViewport(); | ||
| masonryService.changeColumnCount(newColumnCount); | ||
| UnsplashLib.layoutPhotos(); | ||
| const columns = UnsplashLib.getColumns(); | ||
| setDataset(columns || []); | ||
| }; | ||
|
|
||
| window.addEventListener('resize', handleResize); | ||
|
|
||
| // Call once to make sure state is in sync on mount | ||
| handleResize(); | ||
|
|
||
| return () => { | ||
| window.removeEventListener('resize', handleResize); | ||
| }; | ||
| }, [UnsplashLib, masonryService]); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard resize reflow to avoid unnecessary work
On every resize event you reflow masonry and reset dataset even if the column count didn’t change. Add a guard to skip heavy work when the computed count matches the current one.
const handleResize = () => {
- const newColumnCount = calculateColumnCountForViewport();
- masonryService.changeColumnCount(newColumnCount);
- UnsplashLib.layoutPhotos();
- const columns = UnsplashLib.getColumns();
- setDataset(columns || []);
+ const newColumnCount = calculateColumnCountForViewport();
+ // Skip if nothing changes
+ if (newColumnCount === UnsplashLib.getColumnCount()) {
+ return;
+ }
+ masonryService.changeColumnCount(newColumnCount);
+ UnsplashLib.layoutPhotos();
+ const columns = UnsplashLib.getColumns();
+ setDataset(columns || []);
};Optional: throttle via requestAnimationFrame or a small debounce to smooth continuous resizing if needed.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/kg-unsplash-selector/src/UnsplashSearchModal.tsx around lines 91 to
109, the resize handler always triggers a full reflow and setDataset even when
the computed column count hasn't changed; update the handler to compute
newColumnCount, compare it to the current column count (use an existing
masonryService getter or track currentColumnCount in a ref/state), and only call
masonryService.changeColumnCount, UnsplashLib.layoutPhotos, and setDataset when
the counts differ; optionally wrap the work in requestAnimationFrame or a small
debounce to avoid repeated heavy work during continuous resize.
| const selectImg = (payload:Photo) => { | ||
| const isMobileViewport = window.matchMedia('(max-width: 540px)').matches; | ||
| const isTouchDevice = window.matchMedia('(pointer: coarse)').matches; | ||
| const shouldNotZoom = isMobileViewport || isTouchDevice; | ||
|
|
||
| if (shouldNotZoom) { | ||
| setZoomedImg(null); | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
Type mismatch: allow null in selectImg signature
This function is called with null from UnsplashZoomed to exit zoom, but it’s typed as Photo. Update to accept Photo | null to match usage and prevent type bugs.
-const selectImg = (payload:Photo) => {
+const selectImg = (payload: Photo | null) => {📝 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 selectImg = (payload:Photo) => { | |
| const isMobileViewport = window.matchMedia('(max-width: 540px)').matches; | |
| const isTouchDevice = window.matchMedia('(pointer: coarse)').matches; | |
| const shouldNotZoom = isMobileViewport || isTouchDevice; | |
| if (shouldNotZoom) { | |
| setZoomedImg(null); | |
| return; | |
| } | |
| const selectImg = (payload: Photo | null) => { | |
| const isMobileViewport = window.matchMedia('(max-width: 540px)').matches; | |
| const isTouchDevice = window.matchMedia('(pointer: coarse)').matches; | |
| const shouldNotZoom = isMobileViewport || isTouchDevice; | |
| if (shouldNotZoom) { | |
| setZoomedImg(null); | |
| return; | |
| } | |
| // …rest of the handler… |
🤖 Prompt for AI Agents
In packages/kg-unsplash-selector/src/UnsplashSearchModal.tsx around lines 191 to
200, the selectImg function is typed to accept Photo but is invoked with null
from UnsplashZoomed to exit zoom; update the function signature to accept Photo
| null and update internal logic to handle a null payload (e.g., early return or
clearing zoomed state) so TypeScript no longer flags a type mismatch; also
update any local usages/call sites and related type annotations (handlers,
props, or state setters) to accept null where appropriate to keep types
consistent.
1 participant
Summary
The Unsplash modal is nearly unusable on mobile and tablet devices due to layout issues, small interactive elements, and broken hover/touch behaviours. This PR updates the modal to be fully responsive, fixes interaction bugs on touch devices, and ensures UI elements (such as the Unsplash logo) are visible and accessible. These changes improve the UX in both the lexical editor and the design editor (
/settings/design/edit), both of which use the same Unsplash React component.Issues Fixed:
Fixes all bugs reported in TryGhost/Ghost#24645.
Changes:
Screen.Recording.2025-08-11.at.8.48.34.AM.mp4
Screen.Recording.2025-08-11.at.8.55.28.AM.mp4
Search bar:
Unsplash logo:
Testing Instructions
yarn buildinpackages/kg-unsplash-selector/. Next, follow these steps to link Koenig to your local Ghost admin./unsplashto open the Unsplash modal.