mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-13 18:59:42 +02:00
fix(tabs): a found tab still drops onto the tile grid during a rail search (#580)
183efacc turned drag off while the rail search narrows the list, as the
owner asked, by refusing the flat manual rail's dragstart. That also
refused a drag that never reorders anything: with the tile grid open, a
found tab dragged onto a tile or an empty cell set no draggedTabId, so
_acceptTabDrops() (tile-grid.js) saw nothing and the drop was lost. On
the PR head that drop replaced or swapped the tile. The owner's reason
covers the reorder only: a reorder drop is saved for every device,
relative to rows the search hides, while the tile grid is per-device and
lands beside nothing hidden.
dragstart is unconditional again. The rows refuse the reorder instead:
their dragover returns before preventDefault while _tabRailSearchActive()
(the browser shows no-drop and fires no drop there), and their drop
returns before touching sessionOrder, for anything above the row that
might let a drop through. `draggable` stays on, so a cleared search
reorders with the same rows. The grouped rail's pointer drag keeps its
refusal in _onTabLayoutPointerDown(): it has no drop target besides the
rail.
Test: during a search that leaves both rows showing, dragstart is not
cancelled and sets draggedTabId, dragover on another row is not
accepted, a drop there leaves sessionOrder alone and saves nothing, the
real _acceptTabDrops() binder accepts the drag on a tile and hands it
the id, and after clearing the same rows reorder. It fails on the
previous commit, without the dragover guard, and without the drop guard.
CLAUDE.md, the invariants, the Dashboard wiki row and the release
changeset now say reordering by drag is off and a tile drop still works.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -22,6 +22,9 @@
|
||||
* case box on screen (the owner's call on #580).
|
||||
* - Escape in a box that holds text clears the search and closes nothing else:
|
||||
* the global key handler runs first (capture phase) and must route it.
|
||||
* - No reorder by drag while searching (the owner's call): the grouped rail's
|
||||
* drag does not start, and the flat rail's rows refuse the drop, while a
|
||||
* found tab can still be dropped onto a tile.
|
||||
* - A change in what shows redraws the connector lines, which are anchored to
|
||||
* row positions; an unchanged re-apply does not.
|
||||
* - The sidebar hides a case box its filter emptied, like the rail.
|
||||
@@ -60,6 +63,21 @@ type TabSearch = {
|
||||
) => SearchResult;
|
||||
};
|
||||
|
||||
/**
|
||||
* The tile grid's real drop-target binder (`_acceptTabDrops`), lifted out of
|
||||
* tile-grid.js in a context of its own, so the rest of that module (it wraps
|
||||
* the tab renders) is never mixed into the app under test.
|
||||
*/
|
||||
function loadAcceptTabDrops(): (this: unknown, el: unknown, onDrop: (id: string) => void) => void {
|
||||
const context = vm.createContext({ window: {}, document: {} });
|
||||
vm.runInContext(
|
||||
`function CodemanApp() {}\n${read('tile-grid.js')}\n;globalThis.__accept = CodemanApp.prototype._acceptTabDrops;`,
|
||||
context,
|
||||
{ filename: 'tile-grid.js' }
|
||||
);
|
||||
return (context as unknown as { __accept: ReturnType<typeof loadAcceptTabDrops> }).__accept;
|
||||
}
|
||||
|
||||
function loadSearch(): TabSearch {
|
||||
const context = vm.createContext({ window: {}, globalThis: {} });
|
||||
vm.runInContext(read('constants.js'), context, { filename: 'constants.js' });
|
||||
@@ -690,7 +708,7 @@ describe('Escape in the search box', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('no drag while a rail search is active', () => {
|
||||
describe('no reorder by drag while a rail search is active', () => {
|
||||
it('will not start a grouped-rail drag, and starts one again once the search is cleared', () => {
|
||||
const app = makeApp();
|
||||
app._fullRenderSessionTabs();
|
||||
@@ -718,30 +736,56 @@ describe('no drag while a rail search is active', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it('refuses the flat rail drag while searching, though the rows stay bound for after', () => {
|
||||
it('lets a found tab be dragged onto a tile, never dropped onto a rail row, and reorders again once cleared', () => {
|
||||
const app = makeApp({ tabLayout: null });
|
||||
app.saveSessionOrder = vi.fn();
|
||||
app._fullRenderSessionTabs();
|
||||
const row = () => document.querySelector<HTMLElement>('#sessionTabs [data-id="roadmap"]')!;
|
||||
expect(row().getAttribute('draggable')).toBe('true');
|
||||
const dragstart = () => {
|
||||
const event = new window.Event('dragstart', { bubbles: true, cancelable: true });
|
||||
Object.defineProperty(event, 'dataTransfer', { value: { effectAllowed: '', setData: () => {} } });
|
||||
const row = (id: string) => document.querySelector<HTMLElement>(`#sessionTabs [data-id="${id}"]`)!;
|
||||
expect(row('roadmap').getAttribute('draggable')).toBe('true');
|
||||
const drag = (type: string, target: HTMLElement) => {
|
||||
const event = new window.Event(type, { bubbles: true, cancelable: true });
|
||||
Object.defineProperty(event, 'dataTransfer', {
|
||||
value: { effectAllowed: '', dropEffect: '', setData: () => {} },
|
||||
});
|
||||
target.dispatchEvent(event);
|
||||
return event;
|
||||
};
|
||||
// A tile, with the tile grid's own drop-target binder (tile-grid.js).
|
||||
const tile = document.createElement('div');
|
||||
document.body.appendChild(tile);
|
||||
const onTileDrop = vi.fn();
|
||||
loadAcceptTabDrops().call(app, tile, onTileDrop);
|
||||
app._tileGrid = { open: true };
|
||||
|
||||
app.setTabRailSearch('road');
|
||||
const refused = dragstart();
|
||||
row().dispatchEvent(refused);
|
||||
expect(refused.defaultPrevented).toBe(true);
|
||||
expect(app.draggedTabId ?? null).toBeNull();
|
||||
|
||||
// A keystroke does not re-render, so the cleared rail drags with the same rows.
|
||||
app.clearTabRailSearch();
|
||||
const allowed = dragstart();
|
||||
row().dispatchEvent(allowed);
|
||||
expect(allowed.defaultPrevented).toBe(false);
|
||||
// 'r' leaves Roadmap and API Review both showing, so only the search refuses.
|
||||
app.setTabRailSearch('r');
|
||||
expect(visibleRows()).toEqual(['roadmap', 'review', 'web']);
|
||||
expect(drag('dragstart', row('roadmap')).defaultPrevented).toBe(false);
|
||||
expect(app.draggedTabId).toBe('roadmap');
|
||||
row().dispatchEvent(new window.Event('dragend', { bubbles: true }));
|
||||
|
||||
// A rail row is no drop target: no preventDefault (the browser shows no-drop),
|
||||
// no indicator, and a drop that gets through anyway moves nothing.
|
||||
expect(drag('dragover', row('review')).defaultPrevented).toBe(false);
|
||||
expect(row('review').className).not.toMatch(/drag-over/);
|
||||
drag('drop', row('review'));
|
||||
expect(app.sessionOrder).toEqual(['alpha', 'roadmap', 'review', 'notes']);
|
||||
expect(app.saveSessionOrder).not.toHaveBeenCalled();
|
||||
|
||||
// A tile is (per-device, nothing hidden to land beside).
|
||||
expect(drag('dragover', tile).defaultPrevented).toBe(true);
|
||||
drag('drop', tile);
|
||||
expect(onTileDrop).toHaveBeenCalledWith('roadmap');
|
||||
drag('dragend', row('roadmap'));
|
||||
expect(app.draggedTabId ?? null).toBeNull();
|
||||
app._tileGrid = null;
|
||||
|
||||
// A keystroke does not re-render, so the cleared rail reorders with the same rows.
|
||||
app.clearTabRailSearch();
|
||||
drag('dragstart', row('roadmap'));
|
||||
expect(drag('dragover', row('review')).defaultPrevented).toBe(true);
|
||||
drag('drop', row('review'));
|
||||
expect(app.sessionOrder).toEqual(['alpha', 'review', 'roadmap', 'notes']);
|
||||
expect(app.saveSessionOrder).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user