Create Sql-query-runner Dialog#218
Conversation
ivy-tgr
force-pushed
the
XIVY-18080-SQL-Editor
branch
7 times, most recently
from
3d27849 to
aaad906
Compare
ivy-edp
left a comment
ivy-edp
left a comment
There was a problem hiding this comment.
In general okay, however some comments to address/improve
|
|
||
| const tablesQuery = useMeta('meta/databaseTableNames', { ...context, databaseName: database.name }); | ||
|
|
||
| const executeSqlMutation = useMutation({ |
There was a problem hiding this comment.
A custom useFunction hook would make sense here (similar to the one used in the Form Editor), since we now have at least two places in the Database Editor that use this functionality. Extracting it into a hook would simplify calling function endpoints and help avoid duplicated logic.
There was a problem hiding this comment.
Should I also rewrite the existing two function calls so they also use UseFunction?
| /> | ||
| )} | ||
| <Flex direction='row' justifyContent='space-between' alignItems='center' gap={2}> | ||
| <Input |
There was a problem hiding this comment.
I wouldn’t use an input field here. It’s confusing to present something as an input when it’s strictly read-only, since it naturally implies it’s editable. My initial instinct is always to try typing into it.
I assume the main reason an input was used is to make it easy to copy the last successfully executed query. If that’s the case, I think this should be redesigned to clearly indicate that it’s just a read-only display of the query. A copy icon would handle the copying use case more explicitly.
It might be worth looking at how ChatGPT or Claude present code snippets, since they handle this kind of “read-only but copyable” content in a very clear way.
If you like you can also address this issue in a later PR.
There was a problem hiding this comment.
Will do this in a seperate PR
ivy-tgr
force-pushed
the
XIVY-18080-SQL-Editor
branch
from
aaad906 to
4a7efd2
Compare
ivy-edp
left a comment
ivy-edp
left a comment
There was a problem hiding this comment.
Cool, looks good to me 👍 try using Tailwind instead of inline styles and consider using useFunction in the other places where functions are being used as well. Other than that, it's good to go from my side 👍
ivy-tgr
force-pushed
the
XIVY-18080-SQL-Editor
branch
from
4a7efd2 to
0834ed6
Compare
2 participants
No description provided.