Skip to content

history: use double-click for opening links #1534

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.

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

Merged
merged 1 commit into from
Mar 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@
text-decoration: none;
color: inherit;
line-height: var(--row-height);
pointer-events: none;
}

.domain {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ export function useAuxClickHandler() {
const dispatch = useHistoryServiceDispatch();
useEffect(() => {
const handleAuxClick = (event) => {
const anchor = /** @type {HTMLButtonElement|null} */ (event.target.closest('a[href][data-url]'));
const row = /** @type {HTMLDivElement|null} */ (event.target.closest('[aria-selected]'));
const anchor = /** @type {HTMLAnchorElement|null} */ (row?.querySelector('a[href][data-url]'));
const url = anchor?.dataset.url;
if (anchor && url && event.button === 1) {
event.preventDefault();
Expand All @@ -24,5 +25,5 @@ export function useAuxClickHandler() {
return () => {
document.removeEventListener('auxclick', handleAuxClick);
};
}, []);
}, [platformName, dispatch]);
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,28 +16,56 @@ export function useLinkClickHandler() {
const dispatch = useHistoryServiceDispatch();
useEffect(() => {
/**
* Handles click events on the document, intercepting interactions with anchor elements
* that specify both `href` and `data-url` attributes.
* Handles double-click events, and tries to open a link.
*
* @param {MouseEvent} event - The mouse event triggered by a click.
* @returns {void} - No return value.
*/
function clickHandler(event) {
if (!(event.target instanceof Element)) return;
const anchor = /** @type {HTMLAnchorElement|null} */ (event.target.closest('a[href][data-url]'));
if (anchor) {
const url = anchor.dataset.url;
if (!url) return;
function dblClickHandler(event) {
const url = closestUrl(event);
if (url) {
event.preventDefault();
event.stopImmediatePropagation();
const target = eventToTarget(event, platformName);
dispatch({ kind: 'open-url', url, target });
}
}

document.addEventListener('click', clickHandler);
/**
* Handles keydown events, specifically for Space or Enter keys, on anchor links.
*
* @param {KeyboardEvent} event - The keyboard event triggered by a keydown action.
* @returns {void} - No return value.
*/
function keydownHandler(event) {
if (event.key !== 'Enter' && event.key !== ' ') return;
const url = closestUrl(event);
if (url) {
event.preventDefault();
event.stopImmediatePropagation();
const target = eventToTarget(event, platformName);
dispatch({ kind: 'open-url', url, target });
}
}

document.addEventListener('keydown', keydownHandler);
document.addEventListener('dblclick', dblClickHandler);

return () => {
document.removeEventListener('click', clickHandler);
document.removeEventListener('dblclick', dblClickHandler);
document.removeEventListener('keydown', keydownHandler);
};
}, []);
}, [platformName, dispatch]);
}

/**
* @param {KeyboardEvent|MouseEvent} event
* @return {string|null}
*/
function closestUrl(event) {
if (!(event.target instanceof Element)) return null;
const row = /** @type {HTMLDivElement|null} */ (event.target.closest('[aria-selected]'));
const anchor = /** @type {HTMLAnchorElement|null} */ (row?.querySelector('a[href][data-url]'));
const url = anchor?.dataset.url;
return url || null;
}
26 changes: 8 additions & 18 deletions special-pages/pages/history/integration-tests/history.page.js
Original file line number Diff line number Diff line change
Expand Up @@ -174,12 +174,12 @@ export class HistoryTestPage {
}

async opensLinks() {
const { page } = this;
const link = page.locator('a[href][data-url]').nth(0);
await link.click();
await link.click({ modifiers: ['Meta'] });
await link.click({ modifiers: ['Shift'] });
await link.click({ button: 'middle' });
const row = this.main().locator('[aria-selected]').nth(0);
await row.dblclick();
await row.dblclick({ modifiers: ['Meta'] });
await row.dblclick({ modifiers: ['Shift'] });

await row.locator('a').click({ button: 'middle', force: true });
await this._opensMainLink();
}
async _opensMainLink() {
Expand Down Expand Up @@ -302,19 +302,9 @@ export class HistoryTestPage {
* @param {import('../types/history.ts').DeleteRangeResponse} resp
*/
async menuForHistoryEntry(nth, resp) {
const { page } = this;

const cleanup = this._withDialogHandling(resp);
// console.log(data[0].title);
const data = generateSampleData({ count: this.entries, offset: 0 });
const nthItem = data[nth];
const row = page.getByText(nthItem.title);
await row.hover();
await page.locator(`[data-action="entries_menu"][value=${nthItem.id}]`).click();

const calls = await this.mocks.waitForCallCount({ method: 'entries_menu', count: 1 });
expect(calls[0].payload.params).toStrictEqual({ ids: [nthItem.id] });
cleanup();
await this.menuForMultipleHistoryEntries(nth, [nthItem.id], resp);
}

/**
Expand All @@ -329,7 +319,7 @@ export class HistoryTestPage {
// console.log(data[0].title);
const data = generateSampleData({ count: this.entries, offset: 0 });
const nthItem = data[nth];
const row = page.getByText(nthItem.title);
const row = this.main().locator(`[data-history-entry=${nthItem.id}]`);
await row.hover();
await page.locator(`[data-action="entries_menu"][value=${nthItem.id}]`).click();

Expand Down
4 changes: 2 additions & 2 deletions special-pages/shared/handlers.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
/**
* @param {MouseEvent} event
* @param {MouseEvent|KeyboardEvent} event
* @param {ImportMeta['platform']} platformName
* @return {'new-tab' | 'new-window' | 'same-tab'}
*/
export function eventToTarget(event, platformName) {
const isControlClick = platformName === 'macos' ? event.metaKey : event.ctrlKey;
if (isControlClick) {
return 'new-tab';
} else if (event.shiftKey || event.button === 1 /* middle click */) {
} else if (event.shiftKey || ('button' in event && event.button === 1) /* middle click */) {
return 'new-window';
}
return 'same-tab';
Expand Down
Loading