Skip to content

Commit

Permalink
ntp: default to an empty query when 'all' is selected
Browse files Browse the repository at this point in the history
  • Loading branch information
shakyShane committed Feb 10, 2025
1 parent aacf81d commit e46f0df
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 14 deletions.
9 changes: 8 additions & 1 deletion special-pages/pages/history/app/components/SearchForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,14 @@ export function SearchProvider({ children, query = { term: '' } }) {
if (anchor) {
e.preventDefault();
const range = toRange(anchor.dataset.filter);
if (range) {
// todo: where should this rule live?
if (range === 'all') {
searchState.value = {
term: '',
domain: null,
range: null,
};
} else if (range) {
searchState.value = {
term: null,
domain: null,
Expand Down
32 changes: 31 additions & 1 deletion special-pages/pages/history/app/components/Sidebar.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { useSearchContext } from './SearchForm.js';
import { useComputed } from '@preact/signals';
import { useTypedTranslation } from '../types.js';
import { Trash } from '../icons/Trash.js';
import { useTypedTranslationWith } from '../../../new-tab/app/types.js';

/**
* @import json from "../strings.json"
Expand Down Expand Up @@ -69,9 +70,38 @@ export function Sidebar({ ranges }) {
);
}

/**
* Renders an item component with additional properties and functionality.
*
* @param {Object} props
* @param {import('../../types/history.js').Range} props.range The range value used for filtering and identification.
* @param {string} props.title The title or label of the item.
* @param {import("@preact/signals").Signal<import('../../types/history.js').Range|null>} props.current The current state object used to determine active item styling.
*/
function Item({ range, title, current }) {
const { t } = useTypedTranslationWith(/** @type {json} */ ({}));
const label = (() => {
switch (range) {
case 'all':
return t('show_history_all');
case 'today':
case 'yesterday':
case 'monday':
case 'tuesday':
case 'wednesday':
case 'thursday':
case 'friday':
case 'saturday':
case 'sunday':
return t('show_history_for', { range });
case 'older':
return t('show_history_older');
case 'recentlyOpened':
return t('show_history_closed');
}
})();
return (
<a href="#" data-filter={range} class={cn(styles.item, current.value === range && styles.active)}>
<a href="#" aria-label={label} data-filter={range} class={cn(styles.item, current.value === range && styles.active)}>
<span class={styles.icon}>
<img src={iconMap[range]} />
</span>
Expand Down
4 changes: 3 additions & 1 deletion special-pages/pages/history/app/history.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,9 @@ export function paramsToQuery(params) {
const range = toRange(params.get('range'));
const domain = params.get('domain');

if (range) {
if (range === 'all') {
query = { term: '' };
} else if (range) {
query = { range };
} else if (domain) {
query = { domain };
Expand Down
2 changes: 1 addition & 1 deletion special-pages/pages/history/app/mocks/mock-transport.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export function mockTransport() {
return Promise.resolve(response);
}
case 'query': {
console.log('📤 [outoging]: ', JSON.stringify(msg.params));
console.log('📤 [outgoing]: ', JSON.stringify(msg.params));
if ('term' in msg.params.query) {
const { term } = msg.params.query;
if (term !== '') {
Expand Down
16 changes: 16 additions & 0 deletions special-pages/pages/history/app/strings.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,22 @@
"title": "Search",
"note": ""
},
"show_history_all": {
"title": "Show all history",
"note": "Button text for an action that removes all filters and searches, and replaces the list will all history."
},
"show_history_older": {
"title": "Show older history",
"note": "Button that shows older history entries"
},
"show_history_for": {
"title": "Show history for {range}",
"note": "The placeholder {range} in the title will be dynamically replaced with specific date ranges such as 'Today', 'Yesterday', or days of the week like 'Monday'. For example, if the range is set to 'Today', the title will become 'Show history for Today'."
},
"show_history_closed": {
"title": "Show history for Recently Closed Tabs",
"note": "Button that shows history from tabs that were recently closed"
},
"search_your_history": {
"title": "Search your history",
"note": ""
Expand Down
12 changes: 6 additions & 6 deletions special-pages/pages/history/integration-tests/history.page.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,15 +135,15 @@ export class HistoryTestPage {
expect(params).toStrictEqual({ query, limit: 150, offset });
}

/**
* @param {string} linkText
*/
async selectsRange(linkText) {
async selectsToday() {
const { page } = this;
await page.getByRole('link', { name: linkText }).click();
await page.getByLabel('Show history for today').click();
}

async onlyRangeIsShown(s) {}
async selectsAll() {
const { page } = this;
await page.getByLabel('Show all history').click();
}

/**
* @param {string} term
Expand Down
8 changes: 4 additions & 4 deletions special-pages/pages/history/integration-tests/history.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ test.describe('history', () => {
const hp = HistoryTestPage.create(page, workerInfo).withEntries(100);
await hp.openPage({ additional: { q: 'youtube' } });
await hp.didMakeInitialQueries({ term: 'youtube' });
await hp.selectsRange('today');
await hp.selectsToday();
await hp.didMakeNthQuery({ nth: 1, query: { range: 'today' } });
});
test('switches from initial range to term', async ({ page }, workerInfo) => {
Expand Down Expand Up @@ -72,16 +72,16 @@ test.describe('history', () => {
// and assert we're back at the top of the container
await hp.didResetScroll();
});
test.fail('selecting `all` resets to an empty query', async ({ page }, workerInfo) => {
test('selecting `all` resets to an empty query', async ({ page }, workerInfo) => {
const hp = HistoryTestPage.create(page, workerInfo).withEntries(300);
await hp.openPage({});

// start with a fresh range query
await hp.selectsRange('today');
await hp.selectsToday();
await hp.didMakeNthQuery({ nth: 1, query: { range: 'today' } });

// click 'all' (to reset)
await hp.selectsRange('all');
await hp.selectsAll();

// ensure it's a full reset
await hp.didMakeNthQuery({ nth: 2, query: { term: '' } });
Expand Down
16 changes: 16 additions & 0 deletions special-pages/pages/history/public/locales/en/history.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,22 @@
"title": "Search",
"note": ""
},
"show_history_all": {
"title": "Show all history",
"note": "Button text for an action that removes all filters and searches, and replaces the list will all history."
},
"show_history_older": {
"title": "Show older history",
"note": "Button that shows older history entries"
},
"show_history_for": {
"title": "Show history for {range}",
"note": "The placeholder {range} in the title will be dynamically replaced with specific date ranges such as 'Today', 'Yesterday', or days of the week like 'Monday'. For example, if the range is set to 'Today', the title will become 'Show history for Today'."
},
"show_history_closed": {
"title": "Show history for Recently Closed Tabs",
"note": "Button that shows history from tabs that were recently closed"
},
"search_your_history": {
"title": "Search your history",
"note": ""
Expand Down

0 comments on commit e46f0df

Please sign in to comment.