Skip to content
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

feat: Refactor tab link click handler & golfing #2082

Merged
merged 2 commits into from
Nov 23, 2023
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
55 changes: 27 additions & 28 deletions src/components/Search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,32 +6,32 @@ import { SearchResult, type SearchResultComponent } from './SearchResult';

type SectionRefs = Partial<Record<SectionOrderItem, SearchResultComponent>>;

const runSearch = (text: string, section: SectionRefs) => {
const openTabs = section[DEFAULT_SECTION_ORDER[0]];
const bookmarks = section[DEFAULT_SECTION_ORDER[1]];
const history = section[DEFAULT_SECTION_ORDER[2]];
const topSites = section[DEFAULT_SECTION_ORDER[3]];
const recentlyClosed = section[DEFAULT_SECTION_ORDER[4]];
const searchFor = (text: string, sections: SectionRefs) => {
const openTabs = sections[DEFAULT_SECTION_ORDER[0]];
const bookmarks = sections[DEFAULT_SECTION_ORDER[1]];
const history = sections[DEFAULT_SECTION_ORDER[2]];
const topSites = sections[DEFAULT_SECTION_ORDER[3]];
const recentlyClosed = sections[DEFAULT_SECTION_ORDER[4]];

if (history) {
if (text) {
chrome.history.search({ text }, history.update);
chrome.history.search({ text }, history.$$update);
} else {
history.update([]);
history.$$update([]);
}
}

if (bookmarks) {
if (text) {
chrome.bookmarks.search(text, bookmarks.update);
chrome.bookmarks.search(text, bookmarks.$$update);
} else {
bookmarks.update([]);
bookmarks.$$update([]);
}
}

openTabs?.filter(text);
topSites?.filter(text);
recentlyClosed?.filter(text);
openTabs?.$$filter(text);
topSites?.$$filter(text);
recentlyClosed?.$$filter(text);
};

type SearchComponent = HTMLDivElement;
Expand All @@ -55,15 +55,14 @@ const view = h<SearchComponent>(meta.html);
export const Search = (): SearchComponent => {
const root = view;
const refs = collect<Refs>(root, meta.k, meta.d);
const searchref = refs.s;
const section: SectionRefs = {};
const input = refs.s;
const sections: SectionRefs = {};

searchref.oninput = () => runSearch(searchref.value, section);
input.oninput = () => searchFor(input.value, sections);

searchref.onkeyup = (event) => {
input.onkeyup = (event) => {
if (event.key === 'Escape') {
searchref.value = '';
runSearch('', section);
searchFor((input.value = ''), sections);
}
};

Expand All @@ -72,19 +71,19 @@ export const Search = (): SearchComponent => {
const sectionOrder = storage.o ?? DEFAULT_SECTION_ORDER;

sectionOrder.forEach((name) => {
section[name] = append(SearchResult(name), root);
sections[name] = append(SearchResult(name), root);
});

const openTabs = section[DEFAULT_SECTION_ORDER[0]];
const topSites = section[DEFAULT_SECTION_ORDER[3]];
const recentlyClosed = section[DEFAULT_SECTION_ORDER[4]];
const openTabs = sections[DEFAULT_SECTION_ORDER[0]];
const topSites = sections[DEFAULT_SECTION_ORDER[3]];
const recentlyClosed = sections[DEFAULT_SECTION_ORDER[4]];

if (openTabs) {
const updateOpenTabs = () =>
chrome.tabs.query({}, (tabs) => {
openTabs.update(tabs);
if (searchref.value) {
openTabs.filter(searchref.value);
openTabs.$$update(tabs);
if (input.value) {
openTabs.$$filter(input.value);
}
});

Expand Down Expand Up @@ -127,12 +126,12 @@ export const Search = (): SearchComponent => {
}

if (topSites) {
chrome.topSites.get(topSites.update);
chrome.topSites.get(topSites.$$update);
}

if (recentlyClosed) {
chrome.sessions.getRecentlyClosed({}, (sessions) => {
recentlyClosed.update(
recentlyClosed.$$update(
sessions.map((session) => session.tab).filter((tab) => tab),
);
});
Expand Down
50 changes: 21 additions & 29 deletions src/components/SearchResult.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,30 +18,25 @@ interface OpenTabLink extends LinkComponent {

// eslint-disable-next-line func-names
const handleTabClick = function (this: OpenTabLink) {
const { id, windowId } = this.$$data;

// Switch to the clicked tab
void chrome.tabs.update(id, { active: true });
chrome.tabs.getCurrent((currentTab) => {
if (currentTab!.id === this.$$data.id) return;

// Switch active window if the tab isn't in the current window
chrome.windows.getCurrent({}, (currentWindow) => {
if (currentWindow.id !== windowId) {
void chrome.windows.update(windowId, { focused: true });
}
});
// Switch to the clicked tab
void chrome.windows.update(this.$$data.windowId, { focused: true });
void chrome.tabs.update(this.$$data.id, { active: true });

// Close current "new-tab" page
chrome.tabs.getCurrent((currentTab) => {
// Close current "new-tab" page
void chrome.tabs.remove(currentTab!.id!);
});

// Prevent default behaviour; shorter than `event.preventDefault()`
return false;
};

// eslint-disable-next-line @typescript-eslint/no-explicit-any
export type SearchResultComponent<T = any> = HTMLDivElement & {
// eslint-disable-next-line @typescript-eslint/no-invalid-void-type
update: (this: void, newData: T[]) => void;
// eslint-disable-next-line @typescript-eslint/no-invalid-void-type
filter: (this: void, text: string) => void;
$$update: (newData: T[]) => void;
$$filter: (text: string) => void;
};

type Refs = {
Expand All @@ -52,7 +47,7 @@ type Refs = {

const meta = compile(`
<div hidden>
<h2 @t></h2>
<h2>@t</h2>

<div @l></div>

Expand All @@ -66,40 +61,37 @@ export const SearchResult = <T extends LinkProps & TabItem>(
): SearchResultComponent<T> => {
const root = clone<SearchResultComponent<T>>(view);
const refs = collect<Refs>(root, meta.k, meta.d);
const list = refs.l;
const isOpenTabs = sectionName === DEFAULT_SECTION_ORDER[0];
let rawData: T[];
let renderedLength: number;

const renderList = (listData: T[], showCount = DEFAULT_RESULTS_AMOUNT) => {
performance.mark(sectionName);

const partial = isOpenTabs ? listData : listData.slice(0, showCount);
const list = isOpenTabs ? listData : listData.slice(0, showCount);
let index = 0;
let link;

renderedLength = partial.length;
renderedLength = list.length;
root.hidden = !renderedLength;
refs.m.hidden = renderedLength === listData.length;
list.textContent = '';
refs.l.textContent = '';

for (; index < renderedLength; index++) {
link = append(Link(partial[index]), list);
link = append(Link(list[index]), refs.l);
if (isOpenTabs) {
(link as OpenTabLink).$$data = partial[index];
(link as OpenTabLink).$$data = list[index];
link.__click = handleTabClick;
}
}

performance.measure(sectionName, sectionName);
};

root.update = (newData) => {
renderList(newData);
rawData = newData;
};
// eslint-disable-next-line no-return-assign
root.$$update = (newData) => renderList((rawData = newData));

root.filter = (text) =>
root.$$filter = (text) =>
renderList(
rawData.filter((item) =>
(item.title + '[' + item.url)
Expand All @@ -108,7 +100,7 @@ export const SearchResult = <T extends LinkProps & TabItem>(
),
);

refs.t.textContent = sectionName;
refs.t.nodeValue = sectionName;

refs.m.__click = () =>
renderList(rawData, renderedLength + MORE_RESULTS_AMOUNT);
Expand Down
1 change: 1 addition & 0 deletions src/newtab.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Theme loader code must run first
import './theme';

import { append, createFragment } from 'stage1/runtime';
Expand Down
9 changes: 5 additions & 4 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ export const DEFAULT_SECTION_ORDER = [
// Simplified synthetic click event implementation of setupSyntheticEvent() from
// stage1, plus special handling for browser internal links (e.g. chrome://)
// https://github.com/maxmilton/stage1/blob/master/src/events.ts
// eslint-disable-next-line consistent-return
export const handleClick = (event: MouseEvent): void => {
// eslint-disable-next-line @typescript-eslint/no-invalid-void-type, consistent-return
export const handleClick = (event: MouseEvent): void | false => {
let node = event.target as
| (Node & { __click?(event: MouseEvent): void })
| null;
Expand All @@ -32,8 +32,6 @@ export const handleClick = (event: MouseEvent): void => {

// Only apply special handling to non-http links
if (url?.[0] !== 'h') {
event.preventDefault();

// if (link.target === '_blank' || event.ctrlKey) {
if (event.ctrlKey) {
// Open the location in a new tab
Expand All @@ -42,5 +40,8 @@ export const handleClick = (event: MouseEvent): void => {
// Update the location in the current tab
void chrome.tabs.update({ url });
}

// Prevent default behaviour; shorter than `event.preventDefault()`
return false;
}
};
17 changes: 13 additions & 4 deletions test/unit/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,22 @@ describe('DEFAULT_SECTION_ORDER', () => {
// TODO: Add invarient tests for handleClick

describe('handleClick', () => {
test('default prevented on event when url does not start with "h"', () => {
// test.skip('default prevented on event when url does not start with "h"', () => {
// const event = new window.MouseEvent('click');
// // @ts-expect-error - _target is an implementation detail of happy-dom
// event._target = { href: 'chrome://about' };
// expect(event.defaultPrevented).toBe(false);
// handleClick(event);
// expect(event.defaultPrevented).toBe(true);
// });

// returning false on click events is similar to event.preventDefault()
test('handler returns false when url does not start with "h"', () => {
const event = new window.MouseEvent('click');
// @ts-expect-error - _target is an implementation detail of happy-dom
event._target = { href: 'chrome://about' };
expect(event.defaultPrevented).toBe(false);
handleClick(event);
expect(event.defaultPrevented).toBe(true);
const result = handleClick(event);
expect(result).toBe(false);
});

test('default not prevent on event when url starts with "h"', () => {
Expand Down