mirror of
https://github.com/bilawalsidhu/gods-eye-view.git
synced 2026-11-04 10:24:02 +00:00
fix(street-level): four regressions from the merge review
- The selected sequence is drawn past the per-tile caps again, and its highlight takes every loaded part, drawn or not: in a dense tile a clicked Mapillary or Panoramax line kept its cones but lost its line. - photoViewer tries :fullscreen and :popover-open one at a time, each in its own try: Safari before 16.4 parses neither, and the fallback threw on every flat KartaView or Panoramax photo. - A lit Street View off Google 3D says it needs the Google 3D map again, instead of "every provider is switched off". - Chips carry their provider colour again (--chip-color, as on main). Each comes with the test that reproduced it. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
1 parent
4e2e0d6900
commit
fb777c1c93
9 files changed
+157
-17
No files matched your search
@@ -1058,3 +1058,13 @@ test('switching source by params clears a failed lookup’s error; reapplying th
|
||||
second.setParams({ mapillary: false, other: true });
|
||||
assert.equal(second.getUIState().street.error, kept);
|
||||
});
|
||||
|
||||
test('the panel says why a lit Street View draws nothing off Google 3D', async (t) => {
|
||||
const { presentStreetLevelPanel } =
|
||||
await import('../../ui/streetLevelPresentation.js');
|
||||
const { layer, stack } = await withStreetView(t);
|
||||
layer.setProviderEnabled('google', true);
|
||||
stack.switchTo('esri-imagery');
|
||||
const view = presentStreetLevelPanel(layer.getUIState());
|
||||
assert.match(view.meta, /Google Street View needs the Google 3D map/);
|
||||
});
|
||||
@@ -162,18 +162,27 @@ export function createLineCoverage({
|
||||
/**
|
||||
* A tile's sequences, filtered then capped: capping first could leave a
|
||||
* dense tile of newer flat captures with no 360° lines at all. A line past
|
||||
* the point budget is skipped; a shorter one may still fit.
|
||||
* the point budget is skipped; a shorter one may still fit. The selected
|
||||
* sequence is the answer to a click: it is drawn whatever the caps leave
|
||||
* out, and not counted against them.
|
||||
*/
|
||||
function drawnSequences(entry) {
|
||||
const drawn = [];
|
||||
const current = filter();
|
||||
const selectedId = state.sequence.selectedId ?? null;
|
||||
let points = 0;
|
||||
let counted = 0;
|
||||
for (const sequence of entry.sequenceList) {
|
||||
if (drawn.length >= perTileSequenceCap) break;
|
||||
if (!passes(sequence, current)) continue;
|
||||
if (sequence.id === selectedId) {
|
||||
drawn.push(sequence);
|
||||
continue;
|
||||
}
|
||||
if (counted >= perTileSequenceCap) continue;
|
||||
const size = pointsOf(sequence);
|
||||
if (points + size > perTilePointCap) continue;
|
||||
points += size;
|
||||
counted++;
|
||||
drawn.push(sequence);
|
||||
}
|
||||
return drawn;
|
||||
@@ -207,9 +216,14 @@ export function createLineCoverage({
|
||||
state.coverage.highlight = null;
|
||||
}
|
||||
if (!sequenceId || !scene) return;
|
||||
// From every loaded line, drawn or not: a sequence selected after its
|
||||
// tiles were drawn may have been left out by their caps.
|
||||
const current = filter();
|
||||
const parts = [];
|
||||
for (const entry of state.coverage.tiles.values())
|
||||
parts.push(...(entry.sequences.get(sequenceId)?.parts || []));
|
||||
for (const entry of state.coverage.tiles.values()) {
|
||||
const sequence = entry.sequenceList.find(({ id }) => id === sequenceId);
|
||||
if (sequence && passes(sequence, current)) parts.push(...sequence.parts);
|
||||
}
|
||||
const instances = lineInstances(
|
||||
[{ id: sequenceId, parts }],
|
||||
Cesium.Color.fromCssColorString(colors.selected),
|
||||
|
||||
@@ -591,3 +591,34 @@ test('a tile that fails before the terrain table loads is reported at once, neve
|
||||
process.off('unhandledRejection', onUnhandled);
|
||||
}
|
||||
});
|
||||
|
||||
test('the selected sequence is drawn and highlighted even past the per-tile cap', async () => {
|
||||
const { state, source, coverage, centre, viewer } = setup();
|
||||
const line = (n) => [
|
||||
[
|
||||
[centre.lon - 0.001, centre.lat + n * 1e-6],
|
||||
[centre.lon + 0.001, centre.lat + n * 1e-6],
|
||||
],
|
||||
];
|
||||
const sequences = [];
|
||||
for (let i = 0; i < 700; i++)
|
||||
sequences.push({
|
||||
id: `new-${i}`,
|
||||
parts: line(i),
|
||||
capturedAt: 2_000_000_000_000 + i,
|
||||
});
|
||||
sequences.push({
|
||||
id: 'old',
|
||||
parts: line(800),
|
||||
capturedAt: 1_000_000_000_000,
|
||||
});
|
||||
state.sequence.selectedId = 'old';
|
||||
coverage.refresh();
|
||||
source.calls[0].resolve(encodeCoverageTile(centre.tile, { sequences }));
|
||||
await settle();
|
||||
const [entry] = state.coverage.tiles.values();
|
||||
assert.ok(entry.sequences.has('old'), 'the selected line is drawn');
|
||||
coverage.highlight('old');
|
||||
assert.ok(state.coverage.highlight, 'the selected line is highlighted');
|
||||
coverage.clear();
|
||||
});
|
||||
@@ -6,11 +6,17 @@ import {
|
||||
import { imageSize } from './imageSize.js';
|
||||
import { msg, t } from '../../i18n/index.js';
|
||||
|
||||
/** A photo download (tens of kilobytes to a few megabytes). */
|
||||
/** EXPAND puts the viewer's wrap full screen, or (iPhone) opens it as a popover. */
|
||||
const EXPANDED_SELECTOR =
|
||||
'.sl-viewer-wrap:fullscreen, .sl-viewer-wrap:popover-open';
|
||||
/**
|
||||
* EXPAND puts the viewer's wrap full screen, or (iPhone) opens it as a
|
||||
* popover. Each is tried on its own: a browser that cannot parse one
|
||||
* (Safari before 16.4) throws on the selector.
|
||||
*/
|
||||
const EXPANDED_SELECTORS = [
|
||||
'.sl-viewer-wrap:fullscreen',
|
||||
'.sl-viewer-wrap:popover-open',
|
||||
];
|
||||
|
||||
/** A photo download (tens of kilobytes to a few megabytes). */
|
||||
export const IMAGE_TIMEOUT_MS = 15_000;
|
||||
|
||||
/** A picture file or decoded side past these is refused (a photo is a few MB). */
|
||||
@@ -745,12 +751,13 @@ export function createPhotoViewer({
|
||||
|
||||
/** The viewer is expanded (full screen, or the iPhone popover), not docked in the panel. */
|
||||
function isExpanded() {
|
||||
try {
|
||||
return Boolean(root?.closest?.(EXPANDED_SELECTOR));
|
||||
} catch {
|
||||
// A browser without :popover-open has no popover EXPAND either.
|
||||
return Boolean(root?.closest?.('.sl-viewer-wrap:fullscreen'));
|
||||
}
|
||||
return EXPANDED_SELECTORS.some((selector) => {
|
||||
try {
|
||||
return Boolean(root?.closest?.(selector));
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
/** Another photo, or none: a sharper panorama still on its way is not needed. */
|
||||
|
||||
@@ -536,3 +536,38 @@ test('an <img> that turns out too large is refused and let go', async (t) => {
|
||||
await assert.rejects(aborted);
|
||||
assert.equal(images.at(-1).src, '', 'its download stops');
|
||||
});
|
||||
|
||||
test('a browser without :fullscreen / :popover-open (Safari < 16.4) still opens a flat photo and hands Esc on', async () => {
|
||||
let asked = 0;
|
||||
const viewer = createPhotoViewer({
|
||||
providerId: 'test',
|
||||
name: 'Test',
|
||||
sources: SOURCES,
|
||||
getPhoto: async (id) => PHOTOS[id],
|
||||
step: async () => null,
|
||||
onStep() {},
|
||||
onEscape: () => ++asked > 0,
|
||||
load: async (url) => ({ src: url }),
|
||||
});
|
||||
const host = fakeHost();
|
||||
await viewer.mount(host);
|
||||
const root = host.children[0];
|
||||
// Safari 16.0–16.3: neither pseudo-class parses, so closest() throws.
|
||||
root.closest = (selector) => {
|
||||
if (/:fullscreen|:popover-open/.test(selector))
|
||||
throw new SyntaxError(`'${selector}' is not a valid selector`);
|
||||
return null;
|
||||
};
|
||||
let failure = null;
|
||||
try {
|
||||
await viewer.open('f');
|
||||
} catch (error) {
|
||||
failure = error;
|
||||
}
|
||||
assert.equal(failure, null, 'opening a flat photo does not throw');
|
||||
const event = { key: 'Escape' };
|
||||
event.preventDefault = () => {};
|
||||
event.stopPropagation = () => {};
|
||||
assert.doesNotThrow(() => root.fire('keydown', event), 'Esc does not throw');
|
||||
assert.equal(asked, 1, 'Esc reaches the provider');
|
||||
});
|
||||
@@ -28,6 +28,17 @@ export function syncChipGroup(container, chips = [], { before = null } = {}) {
|
||||
`data-toggle-chip chip-${state}${chip.active ? ' active' : ''}`,
|
||||
);
|
||||
set(button, 'textContent', chip.label);
|
||||
// Optional per-chip colour; CSS decides how it is used.
|
||||
const color = chip.color || '';
|
||||
if ((button.dataset.chipColor || '') !== color) {
|
||||
if (color) {
|
||||
button.style.setProperty('--chip-color', color);
|
||||
button.dataset.chipColor = color;
|
||||
} else {
|
||||
button.style.removeProperty('--chip-color');
|
||||
delete button.dataset.chipColor;
|
||||
}
|
||||
}
|
||||
set(button, 'title', chip.title || '');
|
||||
set(button, 'disabled', Boolean(chip.disabled));
|
||||
attribute(button, 'aria-pressed', String(Boolean(chip.active)));
|
||||
|
||||
@@ -31,3 +31,24 @@ test('keyed chips retain focus, reflect state, reorder and remove without redund
|
||||
syncChipGroup(f.container, []);
|
||||
assert.deepEqual(f.container.children, [legend]);
|
||||
});
|
||||
|
||||
test('a chip colour is set once, changed, and cleared through a CSS variable', () => {
|
||||
const f = railFixture(() => {});
|
||||
const calls = [];
|
||||
syncChipGroup(f.container, [{ id: 'm', label: 'M' }]);
|
||||
const chip = f.container.children[0];
|
||||
chip.style.setProperty = (name, value) => calls.push(['set', name, value]);
|
||||
chip.style.removeProperty = (name) => calls.push(['remove', name]);
|
||||
syncChipGroup(f.container, [{ id: 'm', label: 'M' }]);
|
||||
assert.deepEqual(calls, [], 'a chip without a colour never touches style');
|
||||
syncChipGroup(f.container, [{ id: 'm', label: 'M', color: '#05cb63' }]);
|
||||
syncChipGroup(f.container, [{ id: 'm', label: 'M', color: '#05cb63' }]);
|
||||
syncChipGroup(f.container, [{ id: 'm', label: 'M', color: '#a66bff' }]);
|
||||
syncChipGroup(f.container, [{ id: 'm', label: 'M' }]);
|
||||
assert.deepEqual(calls, [
|
||||
['set', '--chip-color', '#05cb63'],
|
||||
['set', '--chip-color', '#a66bff'],
|
||||
['remove', '--chip-color'],
|
||||
]);
|
||||
assert.equal(chip.dataset.chipColor, undefined);
|
||||
});
|
||||
@@ -57,7 +57,15 @@ class FakeNode {
|
||||
return on;
|
||||
},
|
||||
};
|
||||
this.style = {};
|
||||
const style = {};
|
||||
this.style = Object.assign(style, {
|
||||
setProperty: (key, value) => {
|
||||
style[key] = value;
|
||||
},
|
||||
removeProperty: (key) => {
|
||||
delete style[key];
|
||||
},
|
||||
});
|
||||
Object.defineProperty(this, 'className', {
|
||||
get: () => [...names].join(' '),
|
||||
set: (value) => {
|
||||
|
||||
@@ -232,9 +232,12 @@ function presentMeta(state) {
|
||||
const providers = state.providers || [];
|
||||
if (providers.length === 0) return translateHint(state.coverage.hint);
|
||||
if (!state.enabled) return t('Switch Street Level on to draw its coverage.');
|
||||
// A lit source that cannot run on this map is not listed: the core's hint
|
||||
// says which map it needs.
|
||||
if (!providers.some((p) => p.on))
|
||||
return t(
|
||||
'Every provider is switched off: light a chip to draw its coverage.',
|
||||
return (
|
||||
translateHint(state.coverage.hint) ||
|
||||
t('Every provider is switched off: light a chip to draw its coverage.')
|
||||
);
|
||||
// Key-gated with another source ready: say how to use it.
|
||||
if (state.keyRequired && state.keyAlternatives?.length)
|
||||
|
||||
Reference in new issue
Block a user