-
Notifications
You must be signed in to change notification settings - Fork 55
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: UX Refactor - 3 (#42) and SearchBox Optimization(#53) #54
base: master
Are you sure you want to change the base?
Fix: UX Refactor - 3 (#42) and SearchBox Optimization(#53) #54
Conversation
UX Refactor (DiceDB#42): - Add placeholder in search box - Add bottom padding equal to top navbar for balanced UI spacing - Show all command entries when the search box is in empty/initial state. SearchBox Optimization (IMPORTANT): - Fix: SearchBox re-rendering every second due to usePlayground custom hook's timer - Memoize SearchBox to prevent unnecessary updates - Decouple from custom hook's timer updates
@pawansoni007 please attach screenshots for the changes, and fix the build. |
I'll do the needful for the prettier and push the updated commit. Thanks Btw..what screenshots are you seeking for? |
@pawansoni007 , screenshots of the changes you made help us review PRs faster, otherwise it's hard to understand what changed without. testing locally. |
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.
Thank you so much for contributing!
I also had a few suggestions on the UI changes: https://www.loom.com/share/8c34be797b4740ea85a9f8da27e3e603?sid=97c0970e-df58-4999-a110-8e6ec40f1cd7
usePlayground(); | ||
|
||
const MemoizedSearchBox = useMemo(() => <SearchBox />, []); |
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.
This seems like a hack we had to do because we don't have separate components for the 2 sections of playground (The Terminal and the Searchbox).
Can we just create a component for the Left Column of Playground (Terminal + Commands Left etc), and call usePlayground from it? Basically, neither the Searchbox or any of it's parents should care about the time
or commandsLeft
, these should be state of the sibling for Searchbox.
@pawansoni007 could you please address the conflicts? |
Brother, is it okay if i do this on Friday evening, I'm mysrlf feeling bad 'bout keeping it this long, something came up and I got completely engaged into that. |
UX Refactor (#42):
SearchBox Optimization (IMPORTANT): #53