-
Notifications
You must be signed in to change notification settings - Fork 12
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
Fix load on enter #332
Fix load on enter #332
Conversation
…ws show up faster in dashboard)
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
I am calling setSearchResultsLoading() inside the useEffect that has searchResultsLoading as a dependency. Is this bad practice? |
I don't really like the solution. It works but it introduces an unnecessary state I feel like combining it with results.state is probable better but idk how to fully. |
Another problem: On search from landing page to dashboard, it calls DashboardEmpty and then DashboardError before the searchResultsTable Skeletons If I modify:
|
Can revert but lmk what you think of these changes to use Opinion: I don't feel the spinner in the search bar is all that necessary when the whole page changes to a loading state. Current issue (possibly unsolvable): There's also an issue when reloading/opening the dashboard (but not when navigating to it from the homepage) where the |
nvm the spinners grown on me
solved via a new MUI feature the topMenu does still jump around because CSS seems to be rendering it as mobile first and it doesn't update until JS is run. there might be a css solution |
solved! turns out using an actual media query instead of useMediaQuery fixes this. Probably since CSS is faster than JS |
|
Everything else works perfectly. Searches appear faster because there is less lag in between and code-wise it's very simple |
I think #298 caused both of these issues as they are present on that PR, dev, and prod. |
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.
nice! So are all the breakpoints local to each component now?
|
searchbar.tsx is very convoluted So I:
|
…ms by using space) by bringing back the handleKeyDown function for params.inputProps.onKeyDown in Autocomplete's renderInput (bringing it back from before #298)
For this, I think we should leave the current behavior as-is so it the same as clicking ENTER on an autocomplete suggestion |
There is some issue when I click enter twice for the same search terms without change, it just perpetually loads. I think this will be ready for review after I fix that, and map out how searchBar.tsx works |
My fix prevents the search when the terms haven't changed. No need for another batch of API calls imo (and if they really want, they can refresh). Also I'd have to change a few things so that the useEffect triggers properly and all that. Don't think it is needed. |
Searchbar action flows: @TyHil if you want me to put the comments somewhere else instead lmk. |
Can you check if the spinner is colored correctly in dark mode? It was before idr which commit changed it, but you did some fancy stuff w colors so I don't want to mess that up iirc |
Overview
After https://github.com/UTDNebula/utd-trends/pull/298/files, since searching is initiated quicker, user feels some lag while waiting for search results
What Changed
Set the loading state quicker, and add a loading icon in the search button