Skip to content

Koenig: Improve Unsplash Modal Responsiveness #1576

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.

Sign up for GitHub

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

Open
niranjan-uma-shankar wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Koenig: Improve Unsplash Modal Responsiveness #1576

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1c3b2d7
Fixes mobile responsiveness
niranjan-uma-shankar Aug 10, 2025
52e94cc
Handles zoom levels on touch and non-touch devices
niranjan-uma-shankar Aug 10, 2025
26d1316
Extends the search box to full width in small viewports
niranjan-uma-shankar Aug 10, 2025
9154f31
Aligns search bar width with column count rather than tailwind's scre…
niranjan-uma-shankar Aug 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 46 additions & 1 deletion packages/kg-unsplash-selector/src/UnsplashSearchModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ interface UnsplashModalProps {
}

export const UnsplashSearchModal : React.FC<UnsplashModalProps> = ({onClose, onImageInsert, unsplashProviderConfig}) => {
const ONE_COLUMN_WIDTH = 540;
const TWO_COLUMN_WIDTH = 1024;

const unsplashProvider = useMemo(() => {
if (!unsplashProviderConfig) {
return new InMemoryUnsplashProvider();
Expand Down Expand Up @@ -73,6 +76,37 @@ export const UnsplashSearchModal : React.FC<UnsplashModalProps> = ({onClose, onI
}
}, [galleryRef, zoomedImg]);

const calculateColumnCountForViewport = () => {
if (window.matchMedia(`(max-width: ${ONE_COLUMN_WIDTH}px)`).matches) {
return 1;
}

if (window.matchMedia(`(max-width: ${TWO_COLUMN_WIDTH}px)`).matches) {
return 2;
}

return 3;
};

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]);

Comment on lines +91 to +109
Copy link

@coderabbitai coderabbitai bot Aug 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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 loadInitPhotos = React.useCallback(async () => {
if (initLoadRef.current === false || searchTerm.length === 0) {
setDataset([]);
Expand Down Expand Up @@ -155,6 +189,15 @@ export const UnsplashSearchModal : React.FC<UnsplashModalProps> = ({onClose, onI
}, [galleryRef, loadMorePhotos, zoomedImg]);

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;
}

Comment on lines 191 to +200
Copy link

@coderabbitai coderabbitai bot Aug 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Suggested change
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.

if (payload) {
setZoomedImg(payload);
setLastScrollPos(scrollPos);
Expand All @@ -176,10 +219,12 @@ export const UnsplashSearchModal : React.FC<UnsplashModalProps> = ({onClose, onI
}
return (
<UnsplashSelector
closeModal={onClose}
closeModal={onClose}
columnCount={UnsplashLib.getColumnCount()}
handleSearch={handleSearch}
>
<UnsplashGallery
columnCount={UnsplashLib.getColumnCount()}
dataset={dataset}
error={null}
galleryRef={galleryRef}
Expand Down
4 changes: 4 additions & 0 deletions packages/kg-unsplash-selector/src/api/UnsplashService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ export class UnsplashService implements IUnsplashService {
return this.masonryService.getColumns();
}

getColumnCount(): number {
return this.masonryService.columnCount;
}
Comment on lines +48 to +50
Copy link

@coderabbitai coderabbitai bot Aug 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

Suggested change
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.


async updateSearch(term: string) {
let results = await this.photoUseCases.searchPhotos(term);
this.photos = results;
Expand Down
17 changes: 15 additions & 2 deletions packages/kg-unsplash-selector/src/ui/UnsplashGallery.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@ interface UnsplashGalleryColumnsProps {

interface GalleryLayoutProps {
children?: ReactNode;
columnCount?: number;
galleryRef: RefObject<HTMLDivElement>;
isLoading?: boolean;
zoomed?: Photo | null;
}

interface UnsplashGalleryProps extends GalleryLayoutProps {
columnCount?: number;
error?: string | null;
dataset?: Photo[][] | [];
selectImg?: any;
Expand Down Expand Up @@ -78,9 +80,18 @@ const UnsplashGalleryColumns: React.FC<UnsplashGalleryColumnsProps> = (props) =>
};

const GalleryLayout: React.FC<GalleryLayoutProps> = (props) => {
const columnCount = props?.columnCount ?? 0;
const classNames = [
'flex',
'size-full',
'justify-center',
'overflow-auto',
props?.zoomed ? 'pb-10' : '',
columnCount < 3 ? 'px-5' : 'px-20'
].filter(Boolean).join(' ');
return (
<div className="relative h-full overflow-hidden" data-kg-unsplash-gallery>
<div ref={props.galleryRef} className={`flex size-full justify-center overflow-auto px-20 ${props?.zoomed ? 'pb-10' : ''}`} data-kg-unsplash-gallery-scrollref>
<div className={`relative h-full overflow-hidden ${columnCount === 1 ? 'px-5' : ''}`} data-kg-unsplash-gallery>
<div ref={props.galleryRef} className={classNames} data-kg-unsplash-gallery-scrollref>
{props.children}
{props?.isLoading && <UnsplashGalleryLoading />}
</div>
Expand All @@ -89,6 +100,7 @@ const GalleryLayout: React.FC<GalleryLayoutProps> = (props) => {
};

const UnsplashGallery: React.FC<UnsplashGalleryProps> = ({zoomed,
columnCount,
error,
galleryRef,
isLoading,
Expand Down Expand Up @@ -133,6 +145,7 @@ const UnsplashGallery: React.FC<UnsplashGalleryProps> = ({zoomed,

return (
<GalleryLayout
columnCount={columnCount}
galleryRef={galleryRef}
isLoading={isLoading}
zoomed={zoomed}>
Expand Down
10 changes: 5 additions & 5 deletions packages/kg-unsplash-selector/src/ui/UnsplashImage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const UnsplashImage: FC<UnsplashImageProps> = ({payload, srcUrl, links, likes, u

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'}`}
Copy link

@coderabbitai coderabbitai bot Aug 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

style={{backgroundColor: payload.color || 'transparent'}}
data-kg-unsplash-gallery-item
onClick={handleClick}>
Expand Down Expand Up @@ -61,10 +61,10 @@ const UnsplashImage: FC<UnsplashImageProps> = ({payload, srcUrl, links, likes, u
icon="download"
/>
</div>
<div className="flex items-center justify-between">
<div className="flex items-center">
<img alt="author" className="mr-2 size-8 rounded-full" src={user.profile_image.medium} />
<div className="mr-2 truncate font-sans text-sm font-medium text-white">{user.name}</div>
<div className="flex items-center justify-between truncate">
<div className="flex w-0 flex-1 items-center">
<img alt="author" className="mr-2 size-8 shrink-0 rounded-full" src={user.profile_image.medium} />
<div className="truncate font-sans text-sm font-medium text-white">{user.name}</div>
</div>
<UnsplashButton label="Insert image" data-kg-unsplash-insert-button onClick={(e) => {
e.stopPropagation();
Expand Down
9 changes: 5 additions & 4 deletions packages/kg-unsplash-selector/src/ui/UnsplashSelector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@ import {ChangeEvent, FunctionComponent, ReactNode} from 'react';

interface UnsplashSelectorProps {
closeModal: () => void;
columnCount: number;
handleSearch: (e: ChangeEvent<HTMLInputElement>) => void;
children: ReactNode;
}

const UnsplashSelector: FunctionComponent<UnsplashSelectorProps> = ({closeModal, handleSearch, children}) => {
const UnsplashSelector: FunctionComponent<UnsplashSelectorProps> = ({closeModal, columnCount, handleSearch, children}) => {
return (
<>
<div className="fixed inset-0 z-40 h-[100vh] bg-black opacity-60"></div>
Expand All @@ -22,12 +23,12 @@ const UnsplashSelector: FunctionComponent<UnsplashSelectorProps> = ({closeModal,
/>
</button>
<div className="flex h-full flex-col">
<header className="flex shrink-0 items-center justify-between px-20 py-10">
<header className={`flex shrink-0 items-center justify-between px-20 py-10 flex-row ${columnCount < 3 ? 'flex-col gap-3 px-5' : ''}`}>
<h1 className="flex items-center gap-2 font-sans text-3xl font-bold text-black">
<UnsplashIcon className="mb-1" />
<UnsplashIcon className="size-6 mb-1" />
Unsplash
</h1>
<div className="relative w-full max-w-sm">
<div className={`relative w-full ${columnCount < 3 ? '' : 'max-w-sm'}`}>
<SearchIcon className="text-grey-700 absolute left-4 top-1/2 size-4 -translate-y-2" />
<input className="border-grey-300 focus:border-grey-400 h-10 w-full rounded-full border border-solid pl-10 pr-8 font-sans text-md font-normal text-black focus-visible:outline-none" placeholder="Search free high-resolution photos" autoFocus data-kg-unsplash-search onChange={handleSearch} />
</div>
Expand Down
2 changes: 1 addition & 1 deletion packages/kg-unsplash-selector/src/ui/UnsplashZoomed.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ interface UnsplashZoomedProps extends Omit<UnsplashImageProps, 'zoomed'> {

const UnsplashZoomed: FC<UnsplashZoomedProps> = ({payload, insertImage, selectImg, zoomed}) => {
return (
<div className="flex h-full grow basis-0 justify-center" data-kg-unsplash-zoomed onClick={() => selectImg(null)}>
<div className="flex size-full grow basis-0 justify-center" data-kg-unsplash-zoomed onClick={() => selectImg(null)}>
<UnsplashImage
alt={payload.alt_description}
height={payload.height}
Expand Down