Skip to content

fix: issue #482 (Ashigaru) #486

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

Draft
fatherlinux wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

fix: issue #482 (Ashigaru) #486

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
272 changes: 122 additions & 150 deletions frontend/src/components/Sidebar.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ function Sidebar({ tourActive, poi, isLinearPoi, isNewPOI, newOrganization, isNe
const [hasGauges, setHasGauges] = useState(null);
const [servingTaxis, setServingTaxis] = useState([]);
const [isSidebarExpanded, setIsSidebarExpanded] = useState(() => window.innerWidth < 768);
const [mobilePhotosVisible, setMobilePhotosVisible] = useState(false);
const [hasNavigatedPoi, setHasNavigatedPoi] = useState(false);

useEffect(() => {
Expand Down Expand Up @@ -285,7 +284,6 @@ function Sidebar({ tourActive, poi, isLinearPoi, isNewPOI, newOrganization, isNe
const shouldEnterEditMode = (isAdmin && editMode) || isNewPOI;
setIsEditing(shouldEnterEditMode);
setIsSidebarExpanded(isMobile);
setMobilePhotosVisible(false);
// Respect a deep-linked subtab (e.g. /poi/river_levels) instead of forcing Info.
if (!permalinkInfo && !initialSidebarTab) setSidebarTab('view');
} else {
Expand Down Expand Up @@ -708,7 +706,7 @@ function Sidebar({ tourActive, poi, isLinearPoi, isNewPOI, newOrganization, isNe
{isMobile && !isEditing && (
<button
className="sidebar-expand-btn"
onClick={() => { setIsSidebarExpanded(prev => { if (prev) setMobilePhotosVisible(false); return !prev; }); }}
onClick={() => setIsSidebarExpanded(prev => !prev)}
title={isSidebarExpanded ? 'Collapse panel' : 'Expand panel'}
aria-label={isSidebarExpanded ? 'Collapse panel' : 'Expand panel'}
>
Expand All @@ -728,80 +726,67 @@ function Sidebar({ tourActive, poi, isLinearPoi, isNewPOI, newOrganization, isNe
</div>

<div style={{ position: 'relative' }}>
{isMobile && !mobilePhotosVisible && !isEditing ? (
media.length > 0 ? (
{isEditing && linearFeature?.id ? (
<ImageUploader
destinationId={linearFeature.id}
hasImage={!!linearFeature.has_primary_image}
pendingImage={pendingImage}
onPendingImageChange={setPendingImage}
updatedAt={linearFeature.updated_at}
disabled={saving}
isVirtualPoi={false}
user={user}
poiId={linearFeature.id}
onMediaUpdate={handleMediaUpdate}
/>
) : media.length > 0 ? (
<Mosaic media={media} allMedia={allMedia} poiId={linearFeature?.id} user={user} onMediaUpdate={handleMediaUpdate} />
) : user && linearFeature?.id && !mediaLoading ? (
<div className="sidebar-no-media">
<button
className="sidebar-show-photos-btn"
onClick={() => setMobilePhotosVisible(true)}
className="btn-add-first-media"
onClick={() => setUploadModalOpen(true)}
>
Show Photos
+ Add Photo/Video
</button>
) : null
) : (
</div>
) : null}

{isMobile && !isEditing && !permalinkInfo && onNavigate && poiNavigationList && poiNavigationList.length > 1 && media.length > 0 && (
<>
{isEditing && linearFeature?.id ? (
<ImageUploader
destinationId={linearFeature.id}
hasImage={!!linearFeature.has_primary_image}
pendingImage={pendingImage}
onPendingImageChange={setPendingImage}
updatedAt={linearFeature.updated_at}
disabled={saving}
isVirtualPoi={false}
user={user}
poiId={linearFeature.id}
onMediaUpdate={handleMediaUpdate}
/>
) : media.length > 0 ? (
<Mosaic media={media} allMedia={allMedia} poiId={linearFeature?.id} user={user} onMediaUpdate={handleMediaUpdate} />
) : user && linearFeature?.id && !mediaLoading ? (
<div className="sidebar-no-media">
<button
className="btn-add-first-media"
onClick={() => setUploadModalOpen(true)}
>
+ Add Photo/Video
</button>
</div>
) : null}

{isMobile && !isEditing && !permalinkInfo && onNavigate && poiNavigationList && poiNavigationList.length > 1 && media.length > 0 && (
<>
{currentIndex > 0 && (
<button
className="image-nav-btn image-nav-prev"
onTouchEnd={(e) => {
e.preventDefault();
e.stopPropagation();
debouncedNavigate('prev');
}}
onClick={(e) => {
e.preventDefault();
e.stopPropagation();
}}
aria-label="Previous POI"
>
<span className="image-nav-chevron">‹</span>
</button>
)}
{currentIndex < poiNavigationList.length - 1 && (
<button
className="image-nav-btn image-nav-next"
onTouchEnd={(e) => {
e.preventDefault();
e.stopPropagation();
debouncedNavigate('next');
}}
onClick={(e) => {
e.preventDefault();
e.stopPropagation();
}}
aria-label="Next POI"
>
<span className="image-nav-chevron">›</span>
</button>
)}
</>
{currentIndex > 0 && (
<button
className="image-nav-btn image-nav-prev"
onTouchEnd={(e) => {
e.preventDefault();
e.stopPropagation();
debouncedNavigate('prev');
}}
onClick={(e) => {
e.preventDefault();
e.stopPropagation();
}}
aria-label="Previous POI"
>
<span className="image-nav-chevron">‹</span>
</button>
)}
{currentIndex < poiNavigationList.length - 1 && (
<button
className="image-nav-btn image-nav-next"
onTouchEnd={(e) => {
e.preventDefault();
e.stopPropagation();
debouncedNavigate('next');
}}
onClick={(e) => {
e.preventDefault();
e.stopPropagation();
}}
aria-label="Next POI"
>
<span className="image-nav-chevron">›</span>
</button>
)}
</>
)}
Comment on lines +755 to 792

@gemini-code-assist gemini-code-assist Bot Jun 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

On mobile-width screens (such as desktop browsers resized for testing, or mobile devices using pointing devices like a mouse or trackpad), these navigation buttons are completely unresponsive to clicks. This happens because pointing devices do not trigger onTouchEnd, and the onClick handler is a no-op that only prevents default behavior and propagation.

Since debouncedNavigate already implements a 300ms debounce mechanism to prevent rapid/double triggering, we can safely remove the onTouchEnd handler and perform the navigation directly inside the onClick handler. This simplifies the code and ensures pointing devices work correctly on mobile-width screens.

          {isMobile && !isEditing && !permalinkInfo && onNavigate && poiNavigationList && poiNavigationList.length > 1 && media.length > 0 && (
            <>
              {currentIndex > 0 && (
                <button
                  className="image-nav-btn image-nav-prev"
                  onClick={(e) => {
                    e.preventDefault();
                    e.stopPropagation();
                    debouncedNavigate('prev');
                  }}
                  aria-label="Previous POI"
                >
                  <span className="image-nav-chevron">‹</span>
                </button>
              )}
              {currentIndex < poiNavigationList.length - 1 && (
                <button
                  className="image-nav-btn image-nav-next"
                  onClick={(e) => {
                    e.preventDefault();
                    e.stopPropagation();
                    debouncedNavigate('next');
                  }}
                  aria-label="Next POI"
                >
                  <span className="image-nav-chevron">›</span>
                </button>
              )}
            </>
          )}

Expand Down Expand Up @@ -1038,7 +1023,7 @@ function Sidebar({ tourActive, poi, isLinearPoi, isNewPOI, newOrganization, isNe
{isMobile && !isEditing && (
<button
className="sidebar-expand-btn"
onClick={() => { setIsSidebarExpanded(prev => { if (prev) setMobilePhotosVisible(false); return !prev; }); }}
onClick={() => setIsSidebarExpanded(prev => !prev)}
title={isSidebarExpanded ? 'Collapse panel' : 'Expand panel'}
aria-label={isSidebarExpanded ? 'Collapse panel' : 'Expand panel'}
>
Expand All @@ -1058,86 +1043,73 @@ function Sidebar({ tourActive, poi, isLinearPoi, isNewPOI, newOrganization, isNe
</div>

<div style={{ position: 'relative' }}>
{isMobile && !mobilePhotosVisible && !isEditing ? (
media.length > 0 ? (
{permalinkInfo && (sidebarTab === 'news' || sidebarTab === 'events') ? (
permalinkItem ? (
<DetailImage key={permalinkItem.id} imageUrl={permalinkItem.image_url} poiId={permalinkItem.poi_id} alt={permalinkItem.title} />
) : (
<div style={{ height: '180px', marginBottom: '12px' }} />
)
) : isEditing && destination?.id ? (
<ImageUploader
destinationId={destination.id}
hasImage={!!destination.has_primary_image}
pendingImage={pendingImage}
onPendingImageChange={setPendingImage}
updatedAt={destination.updated_at}
disabled={saving}
isVirtualPoi={destination?.poi_roles?.includes('organization') && !destination?.geometry && !destination?.latitude}
user={user}
poiId={destination.id}
onMediaUpdate={handleMediaUpdate}
/>
) : media.length > 0 ? (
<Mosaic media={media} allMedia={allMedia} poiId={destination?.id} user={user} onMediaUpdate={handleMediaUpdate} />
) : user && destination?.id && !mediaLoading ? (
<div className="sidebar-no-media">
<button
className="sidebar-show-photos-btn"
onClick={() => setMobilePhotosVisible(true)}
className="btn-add-first-media"
onClick={() => setUploadModalOpen(true)}
>
Show Photos
+ Add Photo/Video
</button>
) : null
) : (
<>
{permalinkInfo && (sidebarTab === 'news' || sidebarTab === 'events') ? (
permalinkItem ? (
<DetailImage key={permalinkItem.id} imageUrl={permalinkItem.image_url} poiId={permalinkItem.poi_id} alt={permalinkItem.title} />
) : (
<div style={{ height: '180px', marginBottom: '12px' }} />
)
) : isEditing && destination?.id ? (
<ImageUploader
destinationId={destination.id}
hasImage={!!destination.has_primary_image}
pendingImage={pendingImage}
onPendingImageChange={setPendingImage}
updatedAt={destination.updated_at}
disabled={saving}
isVirtualPoi={destination?.poi_roles?.includes('organization') && !destination?.geometry && !destination?.latitude}
user={user}
poiId={destination.id}
onMediaUpdate={handleMediaUpdate}
/>
) : media.length > 0 ? (
<Mosaic media={media} allMedia={allMedia} poiId={destination?.id} user={user} onMediaUpdate={handleMediaUpdate} />
) : user && destination?.id && !mediaLoading ? (
<div className="sidebar-no-media">
<button
className="btn-add-first-media"
onClick={() => setUploadModalOpen(true)}
>
+ Add Photo/Video
</button>
</div>
) : null}
</div>
) : null}

{isMobile && !isEditing && !permalinkInfo && onNavigate && poiNavigationList && poiNavigationList.length > 1 && media.length > 0 && (
<>
{currentIndex > 0 && (
<button
className="image-nav-btn image-nav-prev"
onTouchEnd={(e) => {
e.preventDefault();
e.stopPropagation();
debouncedNavigate('prev');
}}
onClick={(e) => {
e.preventDefault();
e.stopPropagation();
}}
aria-label="Previous POI"
>
<span className="image-nav-chevron">‹</span>
</button>
)}
{currentIndex < poiNavigationList.length - 1 && (
<button
className="image-nav-btn image-nav-next"
onTouchEnd={(e) => {
e.preventDefault();
e.stopPropagation();
debouncedNavigate('next');
}}
onClick={(e) => {
e.preventDefault();
e.stopPropagation();
}}
aria-label="Next POI"
>
<span className="image-nav-chevron">›</span>
</button>
)}
</>
{isMobile && !isEditing && !permalinkInfo && onNavigate && poiNavigationList && poiNavigationList.length > 1 && media.length > 0 && (
<>
{currentIndex > 0 && (
<button
className="image-nav-btn image-nav-prev"
onTouchEnd={(e) => {
e.preventDefault();
e.stopPropagation();
debouncedNavigate('prev');
}}
onClick={(e) => {
e.preventDefault();
e.stopPropagation();
}}
aria-label="Previous POI"
>
<span className="image-nav-chevron">‹</span>
</button>
)}
{currentIndex < poiNavigationList.length - 1 && (
<button
className="image-nav-btn image-nav-next"
onTouchEnd={(e) => {
e.preventDefault();
e.stopPropagation();
debouncedNavigate('next');
}}
onClick={(e) => {
e.preventDefault();
e.stopPropagation();
}}
aria-label="Next POI"
>
<span className="image-nav-chevron">›</span>
</button>
)}
</>
)}
Comment on lines +1078 to 1115

@gemini-code-assist gemini-code-assist Bot Jun 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

On mobile-width screens (such as desktop browsers resized for testing, or mobile devices using pointing devices like a mouse or trackpad), these navigation buttons are completely unresponsive to clicks. This happens because pointing devices do not trigger onTouchEnd, and the onClick handler is a no-op that only prevents default behavior and propagation.

Since debouncedNavigate already implements a 300ms debounce mechanism to prevent rapid/double triggering, we can safely remove the onTouchEnd handler and perform the navigation directly inside the onClick handler. This simplifies the code and ensures pointing devices work correctly on mobile-width screens.

        {isMobile && !isEditing && !permalinkInfo && onNavigate && poiNavigationList && poiNavigationList.length > 1 && media.length > 0 && (
          <>
            {currentIndex > 0 && (
              <button
                className="image-nav-btn image-nav-prev"
                onClick={(e) => {
                  e.preventDefault();
                  e.stopPropagation();
                  debouncedNavigate('prev');
                }}
                aria-label="Previous POI"
              >
                <span className="image-nav-chevron">‹</span>
              </button>
            )}
            {currentIndex < poiNavigationList.length - 1 && (
              <button
                className="image-nav-btn image-nav-next"
                onClick={(e) => {
                  e.preventDefault();
                  e.stopPropagation();
                  debouncedNavigate('next');
                }}
                aria-label="Next POI"
              >
                <span className="image-nav-chevron">›</span>
              </button>
            )}
          </>
        )}

Expand Down
Loading