-
Notifications
You must be signed in to change notification settings - Fork 11
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
feat(Node): rework page #1917
feat(Node): rework page #1917
Conversation
452a4f1
to
acaa30a
Compare
import {useAdditionalNodeProps} from '../useClusterData'; | ||
|
||
export function ExtendedNode({component: NodeComponent}: {component: typeof Node}) { | ||
const {balancer} = useClusterBaseInfo(); |
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 was moved to useNodeDeveloperUIHref
, there is no need for this wrapper anymore
@@ -25,7 +25,6 @@ const routes = { | |||
vDisk: `/${VDISK}`, | |||
storageGroup: `/${STORAGE_GROUP}`, | |||
tablet: `/${TABLET}/:id`, | |||
tabletsFilters: `/tabletsFilters`, |
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.
Old path, there is no page for this path now
acaa30a
to
f1aacd3
Compare
f1aacd3
to
861afab
Compare
src/containers/Node/Node.tsx
Outdated
useRouteMatch<{id: string; activeTab: string}>(routes.node) ?? Object.create(null); | ||
const match = useRouteMatch<{id: string; activeTab: string}>(routes.node); | ||
|
||
// NodeId is always defined here because the page is wrapped with specific route Router |
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.
Let's better add a straightforward check, so no changes in Router api could affect our code.
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.
Fixed
src/containers/Node/Node.tsx
Outdated
|
||
return {activeTabVerified: actualActiveTab, nodeTabs: actualNodeTabs}; | ||
}, [activeTab, node, isDiskPagesAvailable]); | ||
const renderHelmet = () => { |
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.
what if to move every render function to a separate component (like https://github.com/ydb-platform/ydb-embedded-ui/blob/861afab08d02e8ac481ec5d1f36035bd5eed92f3/src/containers/Tablet/Tablet.tsx ) ?
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.
Fixed
Closes #1339
Make node page look similar to other pages
Stand: https://nda.ya.ru/t/h85Fpl3x7BmekR
Before:

After:

CI Results
Test Status: β PASSED
π Full Report
π No changes in tests. π
Bundle Size: π½
Current: 80.21 MB | Main: 80.26 MB
Diff: 0.05 MB (-0.07%)
β Bundle size decreased.
βΉοΈ CI Information