Skip to content

Commit 70a1c30

Browse files
shakyShaneshakyShane
authored
history: error boundary (#1544)
* history: error boundary * fix view transitions * stylint * add separate empty states * remove number
1 parent 59fcdfe commit 70a1c30

File tree

13 files changed

+219
-46
lines changed

13 files changed

+219
-46
lines changed

‎.stylelintrc.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
"ignoreFiles": ["build/**/*.css", "Sources/**/*.css", "docs/**/*.css", "special-pages/pages/**/*/dist/*.css"],
55
"rules": {
66
"csstree/validator": {
7-
"ignoreProperties": ["text-wrap"]
7+
"ignoreProperties": ["text-wrap", "view-transition-name"]
88
},
99
"alpha-value-notation": null,
1010
"at-rule-empty-line-before": null,

‎special-pages/pages/history/app/components/App.jsx‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,3 +75,24 @@ export function App() {
7575
</div>
7676
);
7777
}
78+
79+
export function AppLevelErrorBoundaryFallback({ children }) {
80+
return (
81+
<div class={styles.paddedError}>
82+
<p>{children}</p>
83+
<div class={styles.paddedErrorRecovery}>
84+
You can try to{' '}
85+
<button
86+
onClick={() => {
87+
const current = new URL(window.location.href);
88+
current.search = '';
89+
current.pathname = '';
90+
location.href = current.toString();
91+
}}
92+
>
93+
Reload this page
94+
</button>
95+
</div>
96+
</div>
97+
);
98+
}

‎special-pages/pages/history/app/components/App.module.css‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ body {
4040
grid-area: header;
4141
padding-left: var(--main-padding-left);
4242
padding-right: var(--main-padding-right);
43+
view-transition-name: header;
44+
z-index: 1;
45+
background-color: var(--history-background-color);
4346
}
4447
.search {
4548
justify-self: flex-end;
@@ -106,3 +109,11 @@ body {
106109
}
107110
}
108111
}
112+
113+
.paddedError {
114+
padding: 1rem;
115+
}
116+
117+
.paddedErrorRecovery {
118+
margin-top: 1rem;
119+
}

‎special-pages/pages/history/app/components/Empty.js‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,38 @@ import { h } from 'preact';
22
import { useTypedTranslation } from '../types.js';
33
import cn from 'classnames';
44
import styles from './VirtualizedList.module.css';
5+
import { useQueryContext } from '../global/Providers/QueryProvider.js';
56

67
/**
78
* Empty state component displayed when no results are available
9+
* @param {object} props
10+
* @param {object} props.title
11+
* @param {object} props.text
812
*/
9-
export function Empty() {
10-
const { t } = useTypedTranslation();
13+
export function Empty({ title, text }) {
1114
return (
1215
<div class={cn(styles.emptyState, styles.emptyStateOffset)}>
1316
<div class={styles.icons}>
1417
<img src="icons/backdrop.svg" width={128} height={96} alt="" />
1518
<img src="icons/clock.svg" width={60} height={60} alt="" class={styles.forground} />
1619
</div>
17-
<h2 class={styles.emptyTitle}>{t('empty_title')}</h2>
18-
<p class={styles.emptyText}>{t('empty_text')}</p>
20+
<h2 class={styles.emptyTitle}>{title}</h2>
21+
<p class={styles.emptyText}>{text}</p>
1922
</div>
2023
);
2124
}
25+
26+
/**
27+
* Use the application state to figure out which title+text to use.
28+
*/
29+
export function EmptyState() {
30+
const { t } = useTypedTranslation();
31+
const query = useQueryContext();
32+
const hasSearch = query.value.term !== null && query.value.term.trim().length > 0;
33+
34+
if (hasSearch) {
35+
return <Empty title={t('no_results_title', { term: `"${query.value.term}"` })} text={t('no_results_text')} />;
36+
}
37+
38+
return <Empty title={t('empty_title')} text={t('empty_text')} />;
39+
}

‎special-pages/pages/history/app/components/Results.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { DDG_DEFAULT_ICON_SIZE, OVERSCAN_AMOUNT } from '../constants.js';
33
import { Item } from './Item.js';
44
import styles from './VirtualizedList.module.css';
55
import { VisibleItems } from './VirtualizedList.js';
6-
import { Empty } from './Empty.js';
6+
import { EmptyState } from './Empty.js';
77
import { useSelected, useSelectionState } from '../global/Providers/SelectionProvider.js';
88
import { useHistoryServiceDispatch, useResultsData } from '../global/Providers/HistoryServiceProvider.js';
99
import { useCallback, useEffect } from 'preact/hooks';
@@ -45,7 +45,7 @@ export function ResultsContainer() {
4545
*/
4646
export function Results({ results, selected, onChange }) {
4747
if (results.value.items.length === 0) {
48-
return <Empty />;
48+
return <EmptyState />;
4949
}
5050

5151
/**

‎special-pages/pages/history/app/history.md‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,8 @@ params for a query: (note: can be an empty string!)
7373
"term": "example.com"
7474
},
7575
"offset": 0,
76-
"limit": 50
76+
"limit": 50,
77+
"source": "initial"
7778
}
7879
```
7980

@@ -85,7 +86,8 @@ params for a range, note: the values here will match what you returned from `get
8586
"range": "today"
8687
},
8788
"offset": 0,
88-
"limit": 50
89+
"limit": 50,
90+
"source": "initial"
8991
}
9092
```
9193

‎special-pages/pages/history/app/index.js‎

Lines changed: 82 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { h, render } from 'preact';
22
import { EnvironmentProvider, UpdateEnvironment } from '../../../shared/components/EnvironmentProvider.js';
33

4-
import { App } from './components/App.jsx';
4+
import { App, AppLevelErrorBoundaryFallback } from './components/App.jsx';
55
import { Components } from './components/Components.jsx';
66

77
import enStrings from '../public/locales/en/history.json';
@@ -14,6 +14,7 @@ import { HistoryServiceProvider } from './global/Providers/HistoryServiceProvide
1414
import { Settings } from './Settings.js';
1515
import { SelectionProvider } from './global/Providers/SelectionProvider.js';
1616
import { QueryProvider } from './global/Providers/QueryProvider.js';
17+
import { InlineErrorBoundary } from '../../../shared/components/InlineErrorBoundary.js';
1718

1819
/**
1920
* @param {Element} root
@@ -45,47 +46,54 @@ export async function init(root, messaging, baseEnvironment) {
4546
.withDebounce(baseEnvironment.urlParams.get('debounce'))
4647
.withUrlDebounce(baseEnvironment.urlParams.get('urlDebounce'));
4748

48-
console.log('initialSetup', init);
49-
console.log('environment', environment);
50-
console.log('settings', settings);
49+
if (!window.__playwright_01) {
50+
console.log('initialSetup', init);
51+
console.log('environment', environment);
52+
console.log('settings', settings);
53+
}
5154

52-
const strings =
53-
environment.locale === 'en'
54-
? enStrings
55-
: await fetch(`./locales/${environment.locale}/history.json`)
56-
.then((resp) => {
57-
if (!resp.ok) {
58-
throw new Error('did not give a result');
59-
}
60-
return resp.json();
61-
})
62-
.catch((e) => {
63-
console.error('Could not load locale', environment.locale, e);
64-
return enStrings;
65-
});
55+
/**
56+
* @param {string} message
57+
*/
58+
const didCatchInit = (message) => {
59+
messaging.reportInitException({ message });
60+
};
6661

62+
const strings = await getStrings(environment);
6763
const service = new HistoryService(messaging);
6864
const query = paramsToQuery(environment.urlParams, 'initial');
69-
const initial = await service.getInitial(query);
65+
const initial = await fetchInitial(query, service, didCatchInit);
7066

7167
if (environment.display === 'app') {
7268
render(
73-
<EnvironmentProvider debugState={environment.debugState} injectName={environment.injectName} willThrow={environment.willThrow}>
74-
<UpdateEnvironment search={window.location.search} />
75-
<TranslationProvider translationObject={strings} fallback={enStrings} textLength={environment.textLength}>
76-
<MessagingContext.Provider value={messaging}>
77-
<SettingsContext.Provider value={settings}>
78-
<QueryProvider query={query.query}>
79-
<HistoryServiceProvider service={service} initial={initial}>
80-
<SelectionProvider>
81-
<App />
82-
</SelectionProvider>
83-
</HistoryServiceProvider>
84-
</QueryProvider>
85-
</SettingsContext.Provider>
86-
</MessagingContext.Provider>
87-
</TranslationProvider>
88-
</EnvironmentProvider>,
69+
<InlineErrorBoundary
70+
messaging={messaging}
71+
context={'History view application'}
72+
fallback={(message) => {
73+
return <AppLevelErrorBoundaryFallback>{message}</AppLevelErrorBoundaryFallback>;
74+
}}
75+
>
76+
<EnvironmentProvider
77+
debugState={environment.debugState}
78+
injectName={environment.injectName}
79+
willThrow={environment.willThrow}
80+
>
81+
<UpdateEnvironment search={window.location.search} />
82+
<TranslationProvider translationObject={strings} fallback={enStrings} textLength={environment.textLength}>
83+
<MessagingContext.Provider value={messaging}>
84+
<SettingsContext.Provider value={settings}>
85+
<QueryProvider query={query.query}>
86+
<HistoryServiceProvider service={service} initial={initial}>
87+
<SelectionProvider>
88+
<App />
89+
</SelectionProvider>
90+
</HistoryServiceProvider>
91+
</QueryProvider>
92+
</SettingsContext.Provider>
93+
</MessagingContext.Provider>
94+
</TranslationProvider>
95+
</EnvironmentProvider>
96+
</InlineErrorBoundary>,
8997
root,
9098
);
9199
} else if (environment.display === 'components') {
@@ -99,3 +107,42 @@ export async function init(root, messaging, baseEnvironment) {
99107
);
100108
}
101109
}
110+
111+
/**
112+
* @param {import('../types/history.js').HistoryQuery} query
113+
* @param {HistoryService} service
114+
* @param {(message: string) => void} didCatch
115+
* @returns {Promise<import('./history.service.js').InitialServiceData>}
116+
*/
117+
async function fetchInitial(query, service, didCatch) {
118+
try {
119+
return await service.getInitial(query);
120+
} catch (e) {
121+
console.error(e);
122+
didCatch(e.message || String(e));
123+
return {
124+
ranges: {
125+
ranges: [{ id: 'all', count: 0 }],
126+
},
127+
query: {
128+
info: { query: { term: '' }, finished: true },
129+
results: [],
130+
lastQueryParams: null,
131+
},
132+
};
133+
}
134+
}
135+
136+
/**
137+
* @param {import("../../../shared/environment").Environment} environment
138+
*/
139+
async function getStrings(environment) {
140+
return environment.locale === 'en'
141+
? enStrings
142+
: await fetch(`./locales/${environment.locale}/new-tab.json`)
143+
.then((x) => x.json())
144+
.catch((e) => {
145+
console.error('Could not load locale', environment.locale, e);
146+
return enStrings;
147+
});
148+
}

‎special-pages/pages/history/app/mocks/mock-transport.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,9 @@ function queryResponseFrom(memory, msg) {
218218
if ('term' in msg.params.query) {
219219
const { term } = msg.params.query;
220220
if (term !== '') {
221+
if (term === 'empty' || term.includes('"') || term.includes('<')) {
222+
return asResponse([], msg.params.offset, msg.params.limit);
223+
}
221224
if (term === 'empty') {
222225
return asResponse([], msg.params.offset, msg.params.limit);
223226
}

‎special-pages/pages/history/app/strings.json‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,17 @@
44
"note": "Text shown where there are no remaining history entries"
55
},
66
"empty_text": {
7-
"title": "Page visits will appear once you start browsing.",
7+
"title": "No browsing history yet.",
88
"note": "Placeholder text when there's no results to show"
99
},
10+
"no_results_title": {
11+
"title": "No results found for {term}",
12+
"note": "The placeholder {term} will be dynamically replaced with the search term entered by the user. For example, if the user searches for 'cats', the title will become 'No results found for cats'."
13+
},
14+
"no_results_text": {
15+
"title": "Try searching for a different URL or keywords",
16+
"note": "Placeholder text when a search gave no results."
17+
},
1018
"delete_all": {
1119
"title": "Delete All",
1220
"note": "Text for a button that deletes all items or entries. An additional confirmation dialog will be presented."

‎special-pages/pages/history/integration-tests/history.page.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,19 @@ export class HistoryTestPage {
277277
await page.getByRole('button', { name: 'Delete All', exact: true }).click();
278278
const calls = await this.mocks.waitForCallCount({ method: 'deleteRange', count: 1 });
279279
expect(calls[0].payload.params).toStrictEqual({ range: 'all' });
280+
await this.hasEmptyState();
281+
}
282+
283+
async hasEmptyState() {
284+
const { page } = this;
280285
await expect(page.getByRole('heading', { level: 2, name: 'Nothing to see here!' })).toBeVisible();
286+
await expect(page.getByText('No browsing history yet.')).toBeVisible();
287+
}
288+
289+
async hasNoResultsState() {
290+
const { page } = this;
291+
await expect(page.getByRole('heading', { level: 2, name: 'No results found for "empty"' })).toBeVisible();
292+
await expect(page.getByText('Try searching for a different URL or keywords')).toBeVisible();
281293
}
282294

283295
/**

0 commit comments

Comments
 (0)