-
Notifications
You must be signed in to change notification settings - Fork 14
feat: side panel aka refrigerator for query text in top queries #2134
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
Conversation
3ea2bd5
to
89ed060
Compare
|
fixed |
should fix design review now |
702d798
to
8d69beb
Compare
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.
Pull Request Overview
This PR introduces a side panel ("refrigerator") to display detailed query text and related information in the Top Queries view. Key changes include:
- Updating sorting behavior in Top Queries by accepting an optional initial sort parameter.
- Adding a new drawer component for displaying query details.
- Refactoring row selection and active state highlighting logic in both TopQueriesData and RunningQueriesData.
Reviewed Changes
Copilot reviewed 19 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
src/containers/Tenant/Diagnostics/TopQueries/utils.ts | Adds an optional initialSort parameter and a new utility to create InfoViewer items. |
src/containers/Tenant/Diagnostics/TopQueries/hooks/useSetSelectedTopQueryRowFromParams.ts | Introduces a hook to select a query row based on URL params. |
src/containers/Tenant/Diagnostics/TopQueries/hooks/useGetSelectedRowTableSort.ts | Adds support for retrieving sort order from URL params. |
src/containers/Tenant/Diagnostics/TopQueries/columns/constants.ts | Updates query columns by adding a new 'QueryHash' column. |
src/containers/Tenant/Diagnostics/TopQueries/columns/columns.tsx | Adjusts the ordering of columns for running queries. |
src/containers/Tenant/Diagnostics/TopQueries/TopQueriesData.tsx | Refactors row click handling and integrates the new query details drawer. |
src/containers/Tenant/Diagnostics/TopQueries/TopQueries.tsx | Removes now-unused row click handlers in favor of the new drawer logic. |
src/containers/Tenant/Diagnostics/TopQueries/RunningQueriesData.tsx | Applies similar refactoring as TopQueriesData for consistency in handling selected rows and drawer display. |
src/containers/Tenant/Diagnostics/TopQueries/QueryDetails/* | New files for rendering query details including the drawer content and copy link button. |
src/containers/Drawer/Drawer.tsx | Implements a new drawer container and item wrapper used across query details views. |
src/components/SyntaxHighlighter/YDBSyntaxHighlighter.tsx | Enhances the syntax highlighter with dynamic padding based on clipboard button settings. |
src/components/Search/Search.tsx | Adds support for passing an input ref to the Search component. |
src/containers/Tenant/Diagnostics/Diagnostics.tsx | Wraps page content in the new Drawer container to support drawer overlays. |
Files not reviewed (5)
- src/containers/Drawer/Drawer.scss: Language not supported
- src/containers/Tenant/Diagnostics/Diagnostics.scss: Language not supported
- src/containers/Tenant/Diagnostics/TopQueries/QueryDetails/QueryDetails.scss: Language not supported
- src/containers/Tenant/Diagnostics/TopQueries/TopQueries.scss: Language not supported
- src/containers/Tenant/Diagnostics/TopQueries/i18n/en.json: Language not supported
src/containers/Tenant/Diagnostics/TopQueries/RunningQueriesData.tsx
Outdated
Show resolved
Hide resolved
@artemmufazalov @Raubzeug ping |
withClipboardButton.alwaysVisible | ||
) { | ||
if (withClipboardButton.withLabel) { | ||
paddingStyles = {paddingRight: 80}; |
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.
</div> | ||
</div> | ||
|
||
<Fullscreen> |
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.
Decided to get rid of Fullscreen view, isn't it?
} | ||
|
||
return ( | ||
<div className={b('not-found-container')}> |
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.
Maybe separate component?
decided to merge refrigerator independently #2224 |
dfe1cdc
to
85933b2
Compare
fixed review issues |
Stand redeployed |
src/components/Drawer/Drawer.tsx
Outdated
| {type: 'close'} | ||
| {type: 'copyLink'; link: string} | ||
| {type: 'custom'; node: React.ReactNode; key: string}; | ||
| {type: DrawerControlType.CLOSE} |
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.
why did you decide to use enum here? String has same type protection, but you should not import enum in any place you want to use drawer controls.
|
||
export const NotFoundContainer = ({onClose}: NotFoundContainerProps) => { | ||
return ( | ||
<div className={b('not-found-container')}> |
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.
Flex
?
@@ -0,0 +1,45 @@ | |||
@import '../../../../../styles/mixins.scss'; | |||
|
|||
.kv-query-details { |
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.
lets change prefix to ydb-
|
||
export const QueryDetails = ({queryText, infoItems, onOpenInEditor}: QueryDetailsProps) => { | ||
return ( | ||
<div className={b()}> |
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.
Flex
?
|
||
const renderDrawerContent = React.useCallback(() => { | ||
if (!isDrawerVisible) { | ||
return null; |
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.
As for me this case should be processed in Drawer
component.
"query-details.not-found.title": "Not found", | ||
"query-details.not-found.description": "This query no longer exists", | ||
"query-not-found": "Query Not Found", | ||
"query-not-found.description": "The selected query is no longer available in the current dataset", |
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.
It seems this key is not be used
"query-details.query.title": "Query Text", | ||
"query-details.not-found.title": "Not found", | ||
"query-details.not-found.description": "This query no longer exists", | ||
"query-not-found": "Query Not Found", |
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.
It seems this key is not be used
"query-details.not-found.description": "This query no longer exists", | ||
"query-not-found": "Query Not Found", | ||
"query-not-found.description": "The selected query is no longer available in the current dataset", | ||
"query-details.copy-link": "Copy link to 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.
and this
"query-not-found": "Query Not Found", | ||
"query-not-found.description": "The selected query is no longer available in the current dataset", | ||
"query-details.copy-link": "Copy link to query", | ||
"query-details.link-copied": "Copied to clipboard" |
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.
and this
redeployed stand |
Closes #2079
Stand
CI Results
Test Status:⚠️ FLAKY
📊 Full Report
😟 No changes in tests. 😕
Bundle Size: 🔺
Current: 83.53 MB | Main: 83.47 MB
Diff: +0.06 MB (0.07%)
ℹ️ CI Information