mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-23 07:26:30 +00:00
fix(tours): explain a kept tour without a line, and say the take-over once
Taking a tour kept in a collection into a trip left the user guessing in two places when the tour had no line. The map box stayed an empty grey square that read as a broken map; it now says that there is no line yet, only the waypoints. Why "Line only" is dimmed was only in the tooltip of a disabled button, which never shows on hover, focus or touch; the reason now stands under the two choices, and a disabled tile sits in a span so its tooltip can show as well. A successful take-over said the same sentence twice, as a toast and as the dialog's last word with "Open trip". The toast is gone. The collection strings move out of tours.ts into a tourCollection.ts per locale, keeping their keys, so no locale's tours file grows past the 1000-line limit. The tours locale spec now holds both files to their own counts.
This commit is contained in:
@@ -6,6 +6,7 @@ import {
|
||||
CircleCheck,
|
||||
CopyPlus,
|
||||
Flag,
|
||||
Info,
|
||||
Loader2,
|
||||
Play,
|
||||
Spline,
|
||||
@@ -153,7 +154,11 @@ function TripChoices({ takeOver }: { takeOver: CollectionTourTakeOver }) {
|
||||
);
|
||||
}
|
||||
|
||||
/** One answer to whether the waypoints come along: an icon, a title, a short tooltip, raised once chosen. */
|
||||
/**
|
||||
* One answer to whether the waypoints come along: an icon, a title, a short
|
||||
* tooltip, raised once chosen. A disabled button fires no hover or focus, so
|
||||
* one that cannot be chosen sits in a span that carries its tooltip.
|
||||
*/
|
||||
function PointsTile({
|
||||
value,
|
||||
selected,
|
||||
@@ -171,56 +176,63 @@ function PointsTile({
|
||||
hint: string;
|
||||
onChoose: (value: PointsChoice) => void;
|
||||
}) {
|
||||
return (
|
||||
<Tooltip label={hint}>
|
||||
<button
|
||||
type="button"
|
||||
role="radio"
|
||||
aria-checked={selected}
|
||||
disabled={disabled}
|
||||
onClick={() => onChoose(value)}
|
||||
data-testid={`kept-tour-points-${value}`}
|
||||
className={`flex w-full min-w-0 items-center gap-2.5 rounded-[12px] border px-3 py-2.5 text-start transition-colors focus-visible:outline focus-visible:outline-2 focus-visible:outline-accent disabled:cursor-default disabled:opacity-50 ${selected ? 'border-transparent bg-surface-card shadow-sm ring-2 ring-accent' : 'border-edge-faint bg-surface-secondary enabled:hover:bg-surface-card'}`}
|
||||
const tile = (
|
||||
<button
|
||||
type="button"
|
||||
role="radio"
|
||||
aria-checked={selected}
|
||||
disabled={disabled}
|
||||
onClick={() => onChoose(value)}
|
||||
data-testid={`kept-tour-points-${value}`}
|
||||
className={`flex w-full min-w-0 items-center gap-2.5 rounded-[12px] border px-3 py-2.5 text-start transition-colors focus-visible:outline focus-visible:outline-2 focus-visible:outline-accent disabled:cursor-default disabled:opacity-50 ${selected ? 'border-transparent bg-surface-card shadow-sm ring-2 ring-accent' : 'border-edge-faint bg-surface-secondary enabled:hover:bg-surface-card'}`}
|
||||
>
|
||||
<span
|
||||
className={`grid h-8 w-8 flex-none place-items-center rounded-full ${selected ? 'bg-accent text-accent-text' : 'bg-edge text-content-secondary'}`}
|
||||
>
|
||||
<span
|
||||
className={`grid h-8 w-8 flex-none place-items-center rounded-full ${selected ? 'bg-accent text-accent-text' : 'bg-edge text-content-secondary'}`}
|
||||
>
|
||||
{icon}
|
||||
</span>
|
||||
<span className="min-w-0 flex-1 font-semibold text-content" style={fs(12.5, 'body')}>
|
||||
{title}
|
||||
</span>
|
||||
</button>
|
||||
</Tooltip>
|
||||
{icon}
|
||||
</span>
|
||||
<span className="min-w-0 flex-1 font-semibold text-content" style={fs(12.5, 'body')}>
|
||||
{title}
|
||||
</span>
|
||||
</button>
|
||||
);
|
||||
return <Tooltip label={hint}>{disabled ? <span className="block">{tile}</span> : tile}</Tooltip>;
|
||||
}
|
||||
|
||||
/** The explicit choice about the waypoints; leaving them out is only there for a tour with a line. */
|
||||
/**
|
||||
* The explicit choice about the waypoints; leaving them out is only there for
|
||||
* a tour with a line, and a tour without one says so under the choice rather
|
||||
* than only in a tooltip.
|
||||
*/
|
||||
function PointsChoices({ takeOver }: { takeOver: CollectionTourTakeOver }) {
|
||||
const { t } = useTranslation();
|
||||
const { tour } = takeOver;
|
||||
if (!tour) return null;
|
||||
const lineOnlyHint = tour.has_line ? t('tours.collection.lineOnlyHint') : t('tours.collection.needsPoints');
|
||||
return (
|
||||
<div role="radiogroup" aria-label={t('tours.library.controlPoints')} className="grid grid-cols-2 gap-2">
|
||||
<PointsTile
|
||||
value="with"
|
||||
selected={takeOver.points === 'with'}
|
||||
disabled={tour.waypoint_count === 0 && tour.has_line}
|
||||
icon={<Waypoints size={15} strokeWidth={2} />}
|
||||
title={t('tours.collection.withPoints', { count: tour.waypoint_count })}
|
||||
hint={t('tours.collection.withPointsHint')}
|
||||
onChoose={takeOver.choosePoints}
|
||||
/>
|
||||
<PointsTile
|
||||
value="without"
|
||||
selected={takeOver.points === 'without'}
|
||||
disabled={!tour.has_line}
|
||||
icon={<Spline size={15} strokeWidth={2} />}
|
||||
title={t('tours.collection.lineOnly')}
|
||||
hint={lineOnlyHint}
|
||||
onChoose={takeOver.choosePoints}
|
||||
/>
|
||||
<div className="flex flex-col gap-2">
|
||||
<div role="radiogroup" aria-label={t('tours.library.controlPoints')} className="grid grid-cols-2 gap-2">
|
||||
<PointsTile
|
||||
value="with"
|
||||
selected={takeOver.points === 'with'}
|
||||
disabled={tour.waypoint_count === 0 && tour.has_line}
|
||||
icon={<Waypoints size={15} strokeWidth={2} />}
|
||||
title={t('tours.collection.withPoints', { count: tour.waypoint_count })}
|
||||
hint={t('tours.collection.withPointsHint')}
|
||||
onChoose={takeOver.choosePoints}
|
||||
/>
|
||||
<PointsTile
|
||||
value="without"
|
||||
selected={takeOver.points === 'without'}
|
||||
disabled={!tour.has_line}
|
||||
icon={<Spline size={15} strokeWidth={2} />}
|
||||
title={t('tours.collection.lineOnly')}
|
||||
hint={t('tours.collection.lineOnlyHint')}
|
||||
onChoose={takeOver.choosePoints}
|
||||
/>
|
||||
</div>
|
||||
{!tour.has_line && (
|
||||
<TourNotice icon={<Info size={13} strokeWidth={2} />}>{t('tours.collection.needsPoints')}</TourNotice>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
@@ -238,6 +250,7 @@ function KeptTourPreview({ takeOver }: { takeOver: CollectionTourTakeOver }) {
|
||||
focusKey={String(tour.id)}
|
||||
status={takeOver.detailStatus}
|
||||
failedText={t('tours.library.previewFailed')}
|
||||
emptyText={t('tours.collection.noLine')}
|
||||
/>
|
||||
<RailFacts
|
||||
facts={tourPreviewFacts(tour, tour.waypoint_count, unit, t, {
|
||||
|
||||
@@ -17,7 +17,7 @@ import { TourSaveToCollectionBarButton, TourSaveToCollectionButton } from './Tou
|
||||
import { useCollectionTourTakeOver } from './useCollectionTourTakeOver';
|
||||
import { useSaveTourToCollection } from './useSaveTourToCollection';
|
||||
|
||||
// FE-COLLECTION-TOURS-001 to FE-COLLECTION-TOURS-013
|
||||
// FE-COLLECTION-TOURS-001 to FE-COLLECTION-TOURS-014
|
||||
|
||||
vi.mock('../../Map/MapViewAuto', () => ({
|
||||
MapViewAuto: ({ route }: { route: Array<Array<[number, number]>> }) => (
|
||||
@@ -241,7 +241,8 @@ describe('useCollectionTourTakeOver', () => {
|
||||
name: 'Ridge',
|
||||
});
|
||||
expect(result.current.flow.done).toEqual({ tripId: 6, tripTitle: 'Coast', tourName: 'Ridge' });
|
||||
expect(toast.success).toHaveBeenCalledWith('“Ridge” is now a tour of “Coast”.');
|
||||
// The dialog's last word says it; a toast would only repeat it.
|
||||
expect(toast.success).not.toHaveBeenCalled();
|
||||
act(() => result.current.flow.openTrip());
|
||||
expect(result.current.location.pathname).toBe('/trips/6');
|
||||
expect(result.current.flow.open).toBe(false);
|
||||
@@ -279,11 +280,11 @@ describe('useCollectionTourTakeOver', () => {
|
||||
});
|
||||
|
||||
describe('CollectionTourTakeOverDialog', () => {
|
||||
function Harness() {
|
||||
function Harness({ tour = kept() }: { tour?: CollectionTourItem }) {
|
||||
const flow = useCollectionTourTakeOver(3);
|
||||
return (
|
||||
<>
|
||||
<button type="button" onClick={() => flow.show(kept())}>
|
||||
<button type="button" onClick={() => flow.show(tour)}>
|
||||
open
|
||||
</button>
|
||||
<CollectionTourTakeOverDialog takeOver={flow} />
|
||||
@@ -322,6 +323,28 @@ describe('CollectionTourTakeOverDialog', () => {
|
||||
rerender(<KeptPointsStrip points={DETAIL.points} dimmed />);
|
||||
expect(screen.getByTestId('kept-points')).toHaveClass('opacity-40');
|
||||
});
|
||||
|
||||
it('FE-COLLECTION-TOURS-014: a tour kept without a line says so in the map box and under the choice', async () => {
|
||||
vi.mocked(collectionToursRepo.detail).mockResolvedValue({
|
||||
...DETAIL,
|
||||
tour: kept({ has_line: false }),
|
||||
line: [],
|
||||
});
|
||||
render(<Harness tour={kept({ has_line: false })} />);
|
||||
fireEvent.click(screen.getByText('open'));
|
||||
const preview = screen.getByTestId('kept-tour-preview');
|
||||
expect(await within(preview).findByTestId('tour-preview-no-line')).toHaveTextContent(
|
||||
'No line yet, only its waypoints'
|
||||
);
|
||||
expect(within(preview).queryByTestId('kept-map')).toBeNull();
|
||||
expect(screen.getByText('This tour has no line yet, so its waypoints have to come along.')).toBeInTheDocument();
|
||||
const lineOnly = screen.getByTestId('kept-tour-points-without');
|
||||
expect(lineOnly).toBeDisabled();
|
||||
// A disabled tile sits in a span, so its tooltip still has something to hover.
|
||||
expect(lineOnly.parentElement?.tagName).toBe('SPAN');
|
||||
expect(screen.getByTestId('kept-tour-points-with')).toHaveAttribute('aria-checked', 'true');
|
||||
expect(screen.getByTestId('kept-tour-points-with').parentElement?.tagName).not.toBe('SPAN');
|
||||
});
|
||||
});
|
||||
|
||||
describe('CollectionToursSection', () => {
|
||||
|
||||
@@ -100,8 +100,8 @@ export function useCollectionTourTakeOver(collectionId: number) {
|
||||
include_waypoints: points === 'with',
|
||||
name: chosenName,
|
||||
});
|
||||
// The dialog's last word says where the tour is now, so no toast repeats it.
|
||||
setDone({ tripId: trip.id, tripTitle: trip.title, tourName: result.tour.name });
|
||||
toast.success(t('tours.collection.taken', { name: result.tour.name, trip: trip.title }));
|
||||
} catch (err: unknown) {
|
||||
toast.error(t(failure(err) === 'offline' ? 'tours.collection.offline' : 'tours.collection.takeFailed'));
|
||||
} finally {
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import type { TourLibraryItem } from '@trek/shared';
|
||||
import { ArrowUp, Clock, Loader2, Ruler } from 'lucide-react';
|
||||
import { ArrowUp, Clock, Loader2, Ruler, Spline } from 'lucide-react';
|
||||
import { useMemo } from 'react';
|
||||
import { OFM_POSITRON } from '../../../constants/mapDefaults';
|
||||
import { useTileUrl } from '../../../hooks/useTileUrl';
|
||||
@@ -113,19 +113,23 @@ type PreviewStatus = 'loading' | 'ready' | 'error' | 'offline' | null;
|
||||
/**
|
||||
* A tour's line on a small map, the way a preview shows it before anything is
|
||||
* taken over: drawn once its points are in, a spinner while they load, a short
|
||||
* line of text when they cannot be read.
|
||||
* line of text when they cannot be read, and, where the caller names it, what
|
||||
* a tour without a line has instead, so an empty box never reads as a broken map.
|
||||
*/
|
||||
export function TourLinePreview({
|
||||
line,
|
||||
focusKey,
|
||||
status,
|
||||
failedText,
|
||||
emptyText,
|
||||
}: {
|
||||
line: ReadonlyArray<readonly unknown[]> | undefined;
|
||||
focusKey: string;
|
||||
status: PreviewStatus;
|
||||
/** What a preview that could not be read says; offline says that it is offline. */
|
||||
failedText: string;
|
||||
/** What a tour read in full but without a line says in the box; without it the box stays empty. */
|
||||
emptyText?: string;
|
||||
}) {
|
||||
const { t } = useTranslation();
|
||||
const tileUrl = useTileUrl(OFM_POSITRON);
|
||||
@@ -160,6 +164,18 @@ export function TourLinePreview({
|
||||
{status === 'offline' ? t('tours.library.offline') : failedText}
|
||||
</span>
|
||||
)}
|
||||
{status === 'ready' && route.length === 0 && emptyText && (
|
||||
<span
|
||||
className="absolute inset-0 flex flex-col items-center justify-center gap-2 px-6 text-center text-content-muted"
|
||||
style={fs(12, 'body')}
|
||||
data-testid="tour-preview-no-line"
|
||||
>
|
||||
<span className="grid h-9 w-9 place-items-center rounded-full bg-surface-card text-content-faint shadow-sm ring-1 ring-edge-faint">
|
||||
<Spline size={16} strokeWidth={2} />
|
||||
</span>
|
||||
{emptyText}
|
||||
</span>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user