Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add recent serches and recent chats in Search router #49457
Add recent serches and recent chats in Search router #49457
Changes from 13 commits
0acddec
682a8ec
386a8f7
5ed32cd
0b3fe4b
3799068
2f3b0a5
80370ca
50d4e78
17cda40
7df4c32
48a4296
96e309a
445a031
6e4d2b2
7e509d5
4e439a2
38ba637
e9cba4e
cf57d6e
ab3109a
48510b5
fb0fc7f
26e9026
e5ae868
b5f33da
7b111e8
66fa714
11525fa
994e17f
1b7f6ac
a986ab6
3b9e946
524eb8b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering, if I should add it but in the end I don't know why do we even need to pass
shouldInitialize
. I don't understand why do we need to wait for transitionEnd to initialize some data in context. I also didn't notice any bugs when having this set to true. Maybe you have some more context @rayane-djouahThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This Modal is also outside any ScreenWrapperContext so it does not have
didScreenTransitionEnd
Check warning on line 64 in src/components/Search/SearchRouter/SearchRouter.tsx
GitHub Actions / ESLint check
Check warning on line 64 in src/components/Search/SearchRouter/SearchRouter.tsx
GitHub Actions / Changed files ESLint check
Check failure on line 65 in src/components/Search/SearchRouter/SearchRouter.tsx
GitHub Actions / ESLint check
Check failure on line 65 in src/components/Search/SearchRouter/SearchRouter.tsx
GitHub Actions / Changed files ESLint check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SzymczakJ
this construct
useCallback(debounce(...
will not pass our linting. I know because I faced this problem in the past.Feel free to read more about this lint rule.
The solution will be either to drop
useCallback
for now, and make sure the function inside is not too computation-heavy, or eslint-ignore this line.Alternatively check this article: https://kyleshevlin.com/debounce-and-throttle-callbacks-with-react-hooks/
For the record I think this warning is more like:
than
Check failure on line 1 in src/components/Search/SearchRouter/SearchRouterInput.tsx
GitHub Actions / ESLint check
Check failure on line 1 in src/components/Search/SearchRouter/SearchRouterInput.tsx
GitHub Actions / Changed files ESLint check
Check failure on line 16 in src/components/Search/SearchRouter/SearchRouterInput.tsx
GitHub Actions / ESLint check
Check failure on line 16 in src/components/Search/SearchRouter/SearchRouterInput.tsx
GitHub Actions / Changed files ESLint check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here for the input query:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to do that in this case? If we change it that way the input will be different than a preselected option which may confuse the user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should keep the input values as is (without changing them). This will be solved by autocomplete in Search v2.5
Check failure on line 99 in src/components/Search/SearchRouter/SearchRouterList.tsx
GitHub Actions / ESLint check
Check failure on line 99 in src/components/Search/SearchRouter/SearchRouterList.tsx
GitHub Actions / ESLint check
Check failure on line 99 in src/components/Search/SearchRouter/SearchRouterList.tsx
GitHub Actions / Changed files ESLint check
Check failure on line 99 in src/components/Search/SearchRouter/SearchRouterList.tsx
GitHub Actions / Changed files ESLint check
Check failure on line 105 in src/components/Search/SearchRouter/SearchRouterList.tsx
GitHub Actions / ESLint check
Check failure on line 105 in src/components/Search/SearchRouter/SearchRouterList.tsx
GitHub Actions / Changed files ESLint check
Check failure on line 123 in src/components/Search/SearchRouter/SearchRouterList.tsx
GitHub Actions / ESLint check
Check failure on line 123 in src/components/Search/SearchRouter/SearchRouterList.tsx
GitHub Actions / Changed files ESLint check
Check warning on line 126 in src/components/Search/SearchRouter/SearchRouterList.tsx
GitHub Actions / ESLint check
Check warning on line 126 in src/components/Search/SearchRouter/SearchRouterList.tsx
GitHub Actions / Changed files ESLint check