Skip to content

Commit

Permalink
fixed tabbing when on search results + working test
Browse files Browse the repository at this point in the history
  • Loading branch information
@RayBB
RayBB committed Dec 2, 2024
1 parent 40d218c commit 34cf8ec
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 19 deletions.
19 changes: 12 additions & 7 deletions openlibrary/plugins/openlibrary/js/SearchBar.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ export class SearchBar {
this.$results = this.$component.find('ul.search-results');
this.$facetSelect = this.$component.find('.search-facet-selector select');
this.$barcodeScanner = this.$component.find('#barcode_scanner_link');
this.$searchSubmit = this.$component.find('.search-bar-submit')

/** State */
/** Whether the bar is in collapsible mode */
Expand Down Expand Up @@ -112,20 +113,24 @@ export class SearchBar {
})

this.$results.on('keydown', (e) => {
if (e.key === 'ArrowUp' || (e.key === 'Tab' && e.shiftKey)) {
if (!e.target.previousElementSibling) {
if (e.key === 'ArrowUp' || e.key === 'ArrowDown') {
// On arrow keys focus on the next item unless there is none, then focus on input
const direction = e.key === 'ArrowUp' ? 'previousElementSibling' : 'nextElementSibling';
if (!e.target[direction]) {
this.$input.trigger('focus');
return false;
} else {
$(e.target.previousElementSibling).trigger('focus');
$(e.target[direction]).trigger('focus');
return false;
}
} else if (e.key === 'ArrowDown' || (e.key === 'Tab' && !e.shiftKey)) {
if (!e.target.nextElementSibling) {
this.$input.trigger('focus');
} else if (e.key === 'Tab') {
// On tab, always go to the next selector (instead of next result), like wikipedia
this.clearAutocompletionResults();
if (e.shiftKey) {
this.$facetSelect.trigger('focus');
return false;
} else {
$(e.target.nextElementSibling).trigger('focus');
this.$searchSubmit.trigger('focus');
return false;
}
} else if (e.key === 'Enter') {
Expand Down
80 changes: 68 additions & 12 deletions tests/unit/js/SearchBar.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,31 +27,31 @@ describe('SearchBar', () => {

test('Updates facet from params', () => {
expect(sb.facet.read()).not.toBe('title');
sb.initFromUrlParams({facet: 'title'});
sb.initFromUrlParams({ facet: 'title' });
expect(sb.facet.read()).toBe('title');
});

test('Ignore invalid facets', () => {
const originalValue = sb.facet.read();
sb.initFromUrlParams({facet: 'spam'});
sb.initFromUrlParams({ facet: 'spam' });
expect(sb.facet.read()).toBe(originalValue);
});

test('Sets input value from q param', () => {
sb.initFromUrlParams({q: 'Harry Potter'});
sb.initFromUrlParams({ q: 'Harry Potter' });
expect(sb.$input.val()).toBe('Harry Potter');
});

test('Remove title prefix from q param', () => {
sb.initFromUrlParams({q: 'title:"Harry Potter"', facet: 'title'});
sb.initFromUrlParams({ q: 'title:"Harry Potter"', facet: 'title' });
expect(sb.$input.val()).toBe('Harry Potter');
sb.initFromUrlParams({q: 'title: "Harry"', facet: 'title'});
sb.initFromUrlParams({ q: 'title: "Harry"', facet: 'title' });
expect(sb.$input.val()).toBe('Harry');
});

test('Persists value in url param', () => {
expect(localStorage.getItem('facet')).not.toBe('title');
sb.initFromUrlParams({facet: 'title'});
sb.initFromUrlParams({ facet: 'title' });
expect(localStorage.getItem('facet')).toBe('title');
});
});
Expand All @@ -64,15 +64,15 @@ describe('SearchBar', () => {
afterEach(() => localStorage.clear());

test('Queries are marshalled before submit for titles', () => {
sb.initFromUrlParams({facet: 'title'});
sb.initFromUrlParams({ facet: 'title' });
const spy = sinon.spy(SearchBar, 'marshalBookSearchQuery');
sb.submitForm();
expect(spy.callCount).toBe(1);
spy.restore();
});

test('Form action is updated on submit', () => {
sb.initFromUrlParams({facet: 'title'});
sb.initFromUrlParams({ facet: 'title' });
const originalAction = sb.$form[0].action;
sb.submitForm();
expect(sb.$form[0].action).not.toBe(originalAction);
Expand Down Expand Up @@ -146,7 +146,7 @@ describe('SearchBar', () => {
test('Advanced facet triggers redirect', () => {
const sb = new SearchBar($(DUMMY_COMPONENT_HTML));
const navigateToStub = sandbox.stub(sb, 'navigateTo');
const event = Object.assign(new $.Event(), { target: { value: 'advanced' }});
const event = Object.assign(new $.Event(), { target: { value: 'advanced' } });
sb.handleFacetSelectChange(event);
expect(navigateToStub.callCount).toBe(1);
expect(navigateToStub.args[0]).toEqual(['/advancedsearch']);
Expand All @@ -168,7 +168,7 @@ describe('SearchBar', () => {
test('Title searches tigger autocomplete even if containing title: prefix', () => {
// Stub debounce to avoid have to manipulate time (!)
sandbox.stub(nonjquery_utils, 'debounce').callsFake(fn => fn);
const sb = new SearchBar($(DUMMY_COMPONENT_HTML), {facet: 'title'});
const sb = new SearchBar($(DUMMY_COMPONENT_HTML), { facet: 'title' });
const getJSONStub = sandbox.stub($, 'getJSON');
sb.$input.val('title:"Harry"');
sb.$input.triggerHandler('focus');
Expand All @@ -178,7 +178,7 @@ describe('SearchBar', () => {
test('Focussing on input when empty does not trigger autocomplete', () => {
// Stub debounce to avoid have to manipulate time (!)
sandbox.stub(nonjquery_utils, 'debounce').callsFake(fn => fn);
const sb = new SearchBar($(DUMMY_COMPONENT_HTML), {facet: 'title'});
const sb = new SearchBar($(DUMMY_COMPONENT_HTML), { facet: 'title' });
const getJSONStub = sandbox.stub($, 'getJSON');
sb.$input.val('');
sb.$input.triggerHandler('focus');
Expand Down Expand Up @@ -215,7 +215,7 @@ describe('SearchBar', () => {

test('Autocomplete rendering behavior depends on existing results', () => {
sandbox.stub(nonjquery_utils, 'debounce').callsFake(fn => fn);
const sb = new SearchBar($(DUMMY_COMPONENT_HTML), {facet: 'title'});
const sb = new SearchBar($(DUMMY_COMPONENT_HTML), { facet: 'title' });
const renderSpy = sandbox.spy(sb, 'renderAutocompletionResults');

// Should render when results are empty
Expand All @@ -229,5 +229,61 @@ describe('SearchBar', () => {
sb.$input.triggerHandler('focus');
expect(renderSpy.callCount).toBe(0, 'Should not render when results exist');
});

test('Tabbing from search result focuses search submit button and clears results', () => {
const sb = new SearchBar($(DUMMY_COMPONENT_HTML));

// Add a dummy result and focus on it
sb.$results.append('<li tabindex="0">Test Result</li>');
const $resultItem = sb.$results.children().first();
$resultItem.trigger('focus');

// Spy on the clearAutocompletionResults method
const clearResultsSpy = sandbox.spy(sb, 'clearAutocompletionResults');

// Spy on the focus trigger for search submit
const focusSpy = sandbox.spy(sb.$searchSubmit, 'trigger');

// Simulate tab keydown event on the result item
const tabEvent = $.Event('keydown', { key: 'Tab', shiftKey: false });
$resultItem.trigger(tabEvent);

// Verify clearAutocompletionResults was called
expect(clearResultsSpy.callCount).toBe(1, 'Should clear autocomplete results');

// Verify search submit was focused
expect(focusSpy.calledWith('focus')).toBe(true, 'Should focus search submit button');

// Verify event default was prevented
expect(tabEvent.isDefaultPrevented()).toBe(true, 'Should prevent default tab behavior');
});

test('Shift+tabbing from search result focuses facet select and clears results', () => {
const sb = new SearchBar($(DUMMY_COMPONENT_HTML));

// Add a dummy result and focus on it
sb.$results.append('<li tabindex="0">Test Result</li>');
const $resultItem = sb.$results.children().first();
$resultItem.trigger('focus');

// Spy on the clearAutocompletionResults method
const clearResultsSpy = sandbox.spy(sb, 'clearAutocompletionResults');

// Spy on the focus trigger for facet select
const focusSpy = sandbox.spy(sb.$facetSelect, 'trigger');

// Simulate shift+tab keydown event on the result item
const shiftTabEvent = $.Event('keydown', { key: 'Tab', shiftKey: true });
$resultItem.trigger(shiftTabEvent);

// Verify clearAutocompletionResults was called
expect(clearResultsSpy.callCount).toBe(1, 'Should clear autocomplete results');

// Verify facet select was focused
expect(focusSpy.calledWith('focus')).toBe(true, 'Should focus facet select');

// Verify event default was prevented
expect(shiftTabEvent.isDefaultPrevented()).toBe(true, 'Should prevent default tab behavior');
});
});
});

0 comments on commit 34cf8ec

Please sign in to comment.