-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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
chore: Empty state refactor #31860
base: master
Are you sure you want to change the base?
chore: Empty state refactor #31860
Conversation
Korbit doesn't automatically review large (500+ lines changed) pull requests such as this one. If you want me to review anyway, use |
@@ -113,7 +113,7 @@ const SqlEditorLeftBar = ({ | |||
'schema', | |||
]); | |||
|
|||
const [emptyResultsWithSearch, setEmptyResultsWithSearch] = useState(false); | |||
const [_emptyResultsWithSearch, setEmptyResultsWithSearch] = useState(false); |
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 actually need this?
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 need setEmptyResultsWithSearch
, and are flushing position 1 of the array. Code is a bit cryptic to me here. Not sure why we need setEmptyResultsWithSearch
- making `size` a prop instead of one-component-per-size - fixing up the svg so its themable using `currentColor` - centralizing/fixing up references
0b2a217
to
5640c74
Compare
/testenv up |
@mistercrunch Processing your ephemeral environment request here. |
@mistercrunch Ephemeral environment spinning up at http://54.186.90.161:8080. Credentials are |
size
a prop instead of one-component-per-sizecurrentColor