Skip to content

Commit

Permalink
Revert "feat: make redirects via the input component case-insensitive (
Browse files Browse the repository at this point in the history
…#875)"

This reverts commit 30eefe2.
  • Loading branch information
jkaho committed Feb 8, 2023
1 parent 63dc758 commit 1ff900a
Show file tree
Hide file tree
Showing 6 changed files with 9 additions and 83 deletions.
3 changes: 1 addition & 2 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@
"react/react-in-jsx-scope": "off",

// TODO: We should re-enable this if we think it's worthwhile
"@typescript-eslint/explicit-module-boundary-types": "off",
"no-plusplus": "off"
"@typescript-eslint/explicit-module-boundary-types": "off"
},
"settings": {
"react": {
Expand Down
6 changes: 0 additions & 6 deletions docs/pages/search-ui/input.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -133,12 +133,6 @@ function Example() {
}
```
### Note
The `Input` component treats redirects as _case-insensitive_, meaning the user-inputted value `Computer` returns the redirect for the query `computer` and vice versa.
The component does not support the handling of multiple redirects for queries of the same word with varied letter casing. For example, if you have created redirects for each of the queries `computer`, `Computer`, and `COMPUTER`, only one of them is used (dependent upon your browser).
## Results
```jsx
Expand Down
44 changes: 4 additions & 40 deletions packages/search-ui/src/Input/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,6 @@ const redirectTarget = {
token:
'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJwdXJwb3NlIjoic2VhcmNoIiwiZGVzdGluYXRpb24iOiJodHRwOi8vdGFyZ2V0LmNvbS5hdS9zaGVldHMiLCJ2YWxzIjp7ImNvbGxlY3Rpb24iOlsiYmVzdGJ1eSJdLCJpZGVudGlmaWVyIjpbInJlZGlyZWN0Il0sInByb2plY3QiOlsiMTU5NDE1MzcxMTkwMTcyNDIyMCJdLCJxIjpbInNoZWV0cyJdLCJxLmlkIjpbIjc2ZGJiOGU2LWE3MDctNDU2NC1iYTYxLWY0NjNiYTRhZDdlYSJdLCJxLnVpZCI6WyI3NmRiYjhlNi1hNzA3LTQ1NjQtYmE2MS1mNDYzYmE0YWQ3ZWEwIl0sInJlZGlyZWN0LkNvbmRpdGlvbiI6WyJxIH4gJ3NoZWV0cyciXSwicmVkaXJlY3QuSUQiOlsiMjJ3MFZGdkdWYVlhQ1Jzc0NBUm11YkQ2bGdUIl0sInJlZGlyZWN0LlRhcmdldCI6WyJodHRwOi8vdGFyZ2V0LmNvbS5hdS9zaGVldHMiXX19.BhcAVPB4z9LjlIoV42CUaEW-H0qCJ2JKngs6OGAXTf8',
};
const redirectTarget2 = {
id: '12w0VFvGVaYaCRssCARmubD6lgA',
target: 'http://target.com.au/sheets2',
token:
'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJwdXJwb3NlIjoic2VhcmNoIiwiZGVzdGluYXRpb24iOiJodHRwOi8vdGFyZ2V0LmNvbS5hdS9zaGVldHMiLCJ2YWxzIjp7ImNvbGxlY3Rpb24iOlsiYmVzdGJ1eSJdLCJpZGVudGlmaWVyIjpbInJlZGlyZWN0Il0sInByb2plY3QiOlsiMTU5NDE1MzcxMTkwMTcyNDIyMCJdLCJxIjpbInNoZWV0cyJdLCJxLmlkIjpbIjc2ZGJiOGU2LWE3MDctNDU2NC1iYTYxLWY0NjNiYTRhZDdlYSJdLCJxLnVpZCI6WyI3NmRiYjhlNi1hNzA3LTQ1NjQtYmE2MS1mNDYzYmE0YWQ3ZWEwIl0sInJlZGlyZWN0LkNvbmRpdGlvbiI6WyJxIH4gJ3NoZWV0cyciXSwicmVkaXJlY3QuSUQiOlsiMjJ3MFZGdkdWYVlhQ1Jzc0NBUm11YkQ2bGdUIl0sInJlZGlyZWN0LlRhcmdldCI6WyJodHRwOi8vdGFyZ2V0LmNvbS5hdS9zaGVldHMiXX19.BhcAVPB4z9LjlIoV42CUaEW-H0qCJ2JKngs6OGAXTf9',
};
const iPhoneResult = {
values: {
_id: {
Expand Down Expand Up @@ -55,11 +49,10 @@ const server = setupServer(
values: {
q: 'sheets',
'q.original': 'sheets',
'q.suggestions': 'suggestion',
'q.suggestions': 'sheets',
},
redirects: {
sheets: redirectTarget,
Sheets: redirectTarget2,
},
searchResponse: {
reads: '141',
Expand All @@ -84,7 +77,6 @@ describe('Input', () => {
});
afterEach(() => {
jest.clearAllMocks();
jest.resetAllMocks();
server.resetHandlers();
});
afterAll(() => {
Expand All @@ -100,12 +92,12 @@ describe('Input', () => {
const input = screen.getByTestId('mysearch');
input.focus(); // need this else we get TypeError: element.ownerDocument.getSelection is not a function
await user.keyboard('sheets');
await waitFor(() => expect(screen.getByRole('option')).toHaveTextContent('suggestion'));
await waitFor(() => expect(screen.getByText('sheets')).toBeInTheDocument());
await user.keyboard('{enter}');

expect(onRedirectSpy).toHaveBeenCalledWith({
...redirectTarget2,
token: `https://re.sajari.com/token/${redirectTarget2.token}`, // sdk-js appends the clickTokenURL
...redirectTarget,
token: `https://re.sajari.com/token/${redirectTarget.token}`, // sdk-js prepends the clickTokenURL
});
});

Expand Down Expand Up @@ -164,31 +156,3 @@ describe('Input', () => {
await waitFor(() => expect(input.attributes.getNamedItem('readonly')).toBeNull());
});
});

describe('redirects', () => {
beforeAll(() => {
server.listen({ onUnhandledRequest: 'warn' });
});
afterAll(() => {
jest.restoreAllMocks();
server.close();
});

it('ignores letter casing of input query on redirect lookup', async () => {
const onRedirectSpy = jest.spyOn(eventTrackingPipeline.getTracking(), 'onRedirect');
customRender(<Input data-testid="mysearch" mode="suggestions" />, {
search: { pipeline: eventTrackingPipeline },
});
const input = screen.getByTestId('mysearch');
input.focus(); // need this else we get TypeError: element.ownerDocument.getSelection is not a function
await userEvent.type(input, 'ShEeTs');
await waitFor(() => expect(screen.getByRole('option')).toHaveTextContent('suggestion'));
await userEvent.keyboard('{enter}');
await waitFor(() => {
expect(onRedirectSpy).toHaveBeenCalledWith({
...redirectTarget2,
token: `https://re.sajari.com/token/${redirectTarget2.token}`, // sdk-js appends the clickTokenURL
});
});
});
});
10 changes: 4 additions & 6 deletions packages/search-ui/src/Input/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { useSearchUIContext } from '../ContextProvider';
import { ResultValues } from '../Results/types';
import mapResultFields from '../utils/mapResultFields';
import { InputProps } from './types';
import { lowercaseObjectKeys } from './utils';

const Input = React.forwardRef((props: InputProps<any>, ref: React.ForwardedRef<HTMLInputElement>) => {
const { t } = useTranslation('input');
Expand All @@ -37,9 +36,8 @@ const Input = React.forwardRef((props: InputProps<any>, ref: React.ForwardedRef<
redirects,
searching: autocompleteSearching,
} = useAutocomplete();
const lowercasedRedirects = lowercaseObjectKeys(redirects);
const redirectsRef = useRef(lowercasedRedirects);
redirectsRef.current = lowercasedRedirects;
const redirectsRef = useRef(redirects);
redirectsRef.current = redirects;
const { customClassNames, disableDefaultStyles = false, tracking } = useSearchUIContext();
const { query } = useQuery();
const [internalSuggestions, setInternalSuggestions] = useState<Array<any>>([]);
Expand Down Expand Up @@ -176,7 +174,7 @@ const Input = React.forwardRef((props: InputProps<any>, ref: React.ForwardedRef<
if (!retainFilters) {
resetFilters();
}
const redirectValue = redirectsRef.current[value.toLowerCase()];
const redirectValue = redirectsRef.current[value];
if (!disableRedirects && redirectValue) {
tracking.onRedirect(redirectValue);
window.location.assign(redirectValue.token || redirectValue.target);
Expand All @@ -185,7 +183,7 @@ const Input = React.forwardRef((props: InputProps<any>, ref: React.ForwardedRef<
// If we're performing an autocomplete search, wait a tick to recheck redirects before unloading
e.preventDefault();
setTimeout(() => {
const redirectTarget = redirectsRef.current[value.toLowerCase()];
const redirectTarget = redirectsRef.current[value];
if (redirectTarget) {
tracking.onRedirect(redirectTarget);
window.location.assign(redirectTarget.token || redirectTarget.target);
Expand Down
20 changes: 0 additions & 20 deletions packages/search-ui/src/Input/utils/index.test.ts

This file was deleted.

9 changes: 0 additions & 9 deletions packages/search-ui/src/Input/utils/index.ts

This file was deleted.

0 comments on commit 1ff900a

Please sign in to comment.