🐛 Do not update querystring on search

After trying this out I think its not a great pattern. If we want to implement this we need to make it onBlur or onSubmit, but definitely not onChange. This causes a weird bug where the page is scrolled to the top since were using router.push and also affects browser navigateBack history. Since the push method modifies the history.pushState.
This commit is contained in:
Carlos Cuesta 2021-01-07 14:19:13 +01:00
parent a49e94640c
commit ab6c193512
No known key found for this signature in database
GPG Key ID: 97D6BDA5F0B12E0D
3 changed files with 20 additions and 51 deletions

View File

@ -12,7 +12,6 @@ jest.mock('next/router', () => ({
events: { on: jest.fn(), off: jest.fn() },
useRouter: jest.fn().mockImplementation(() => ({
query: {},
push: jest.fn(),
})),
}))

View File

@ -38,7 +38,7 @@ describe('GitmojiList', () => {
Router.useRouter.mockReturnValue(stubs.routerMock())
})
it('should find the fire gitmoji by code and update router.query', () => {
it('should find the fire gitmoji by code', () => {
const wrapper = renderer.create(<GitmojiList {...stubs.props} />)
const instance = wrapper.root
const query = 'Fire'
@ -49,15 +49,10 @@ describe('GitmojiList', () => {
.props.onChange({ target: { value: query } })
})
expect(Router.useRouter().push).toHaveBeenLastCalledWith(
{ query: { search: query } },
undefined,
{ shallow: true }
)
expect(instance.findAllByType('article').length).toEqual(1)
})
it('should find the fire gitmoji by description and update router.query', () => {
it('should find the fire gitmoji by description', () => {
const wrapper = renderer.create(<GitmojiList {...stubs.props} />)
const instance = wrapper.root
const query = 'remove'
@ -68,43 +63,10 @@ describe('GitmojiList', () => {
.props.onChange({ target: { value: query } })
})
expect(Router.useRouter().push).toHaveBeenLastCalledWith(
{ query: { search: query } },
undefined,
{ shallow: true }
)
expect(instance.findAllByType('article').length).toEqual(1)
})
})
describe('when user deletes the search query', () => {
beforeAll(() => {
Router.useRouter.mockReturnValue(stubs.routerMock())
})
it('should clear the router.query', () => {
const wrapper = renderer.create(<GitmojiList {...stubs.props} />)
const instance = wrapper.root
const query = 'Fire'
renderer.act(() => {
instance
.findByType('input')
.props.onChange({ target: { value: query } })
})
renderer.act(() => {
instance.findByType('input').props.onChange({ target: { value: '' } })
})
expect(Router.useRouter().push).toHaveBeenLastCalledWith(
{ query: {} },
undefined,
{ shallow: true }
)
})
})
describe('when search is provided by query string', () => {
beforeAll(() => {
Router.useRouter.mockReturnValue(stubs.routerMock({ search: 'fire' }))
@ -118,12 +80,22 @@ describe('GitmojiList', () => {
wrapper.update(<GitmojiList {...stubs.props} />)
})
expect(Router.useRouter().push).toHaveBeenCalledWith(
{ query: { search: query } },
undefined,
{ shallow: true }
)
expect(wrapper.root.findByType('input').props.value).toEqual(query)
})
describe('when the user deletes the search input', () => {
it('should clear the query string', () => {
const wrapper = renderer.create(<GitmojiList {...stubs.props} />)
const instance = wrapper.root
renderer.act(() => {
instance.findByType('input').props.onChange({ target: { value: '' } })
})
expect(Router.useRouter().push).toHaveBeenCalledWith('/', undefined, {
shallow: true,
})
})
})
})
})

View File

@ -38,11 +38,9 @@ const GitmojiList = (props: Props): Element<'div'> => {
}, [router.query.search])
React.useEffect(() => {
router.push(
{ query: searchInput ? { search: searchInput } : {} },
undefined,
{ shallow: true }
)
if (router.query.search && !searchInput) {
router.push('/', undefined, { shallow: true })
}
}, [searchInput])
React.useEffect(() => {