-
Notifications
You must be signed in to change notification settings - Fork 414
RI-7463 sentinel databases rerender #5126
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
base: main
Are you sure you want to change the base?
Conversation
# Conflicts: # redisinsight/ui/src/components/connectivity-error/ConnectivityError.tsx # redisinsight/ui/src/components/inline-item-editor/InlineItemEditor.tsx # redisinsight/ui/src/components/item-list/components/delete-action/DeleteAction.tsx # redisinsight/ui/src/components/item-list/components/export-action/ExportAction.tsx # redisinsight/ui/src/components/query/query-actions/QueryActions.tsx # redisinsight/ui/src/components/query/query-card/QueryCardHeader/QueryCardHeader.tsx # redisinsight/ui/src/components/side-panels/panels/enablement-area/EnablementArea/components/InternalPage/InternalPage.tsx # redisinsight/ui/src/pages/browser/components/add-key/AddKeyHash/AddKeyHash.tsx # redisinsight/ui/src/pages/browser/components/add-key/AddKeyList/AddKeyList.tsx # redisinsight/ui/src/pages/browser/components/add-key/AddKeyReJSON/AddKeyReJSON.tsx # redisinsight/ui/src/pages/browser/components/add-key/AddKeySet/AddKeySet.tsx # redisinsight/ui/src/pages/browser/components/add-key/AddKeyStream/AddKeyStream.tsx # redisinsight/ui/src/pages/browser/components/add-key/AddKeyString/AddKeyString.tsx # redisinsight/ui/src/pages/browser/components/add-key/AddKeyZset/AddKeyZset.tsx # redisinsight/ui/src/pages/browser/modules/key-details/components/rejson-details/components/edit-entire-item-action/EditEntireItemAction.tsx # redisinsight/ui/src/pages/browser/modules/key-details/components/set-details/add-set-members/AddSetMembers.tsx # redisinsight/ui/src/pages/database-analysis/components/header/Header.tsx # redisinsight/ui/src/pages/home/components/database-list-header/DatabaseListHeader.tsx # redisinsight/ui/src/pages/home/components/database-manage-tags-modal/ManageTagsModal.tsx # redisinsight/ui/src/pages/settings/components/cloud-settings/CloudSettings.tsx # redisinsight/ui/src/pages/settings/components/cloud-settings/components/user-api-keys-table/UserApiKeysTable.tsx # redisinsight/ui/src/pages/slow-log/components/Actions/Actions.tsx
| export type PaddingType = | ||
| | keyof typeof flexItemStyles.padding | ||
| | null | ||
| | undefined | ||
| | true | ||
| | 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.
what would a padding: true mean? why are we allowing this as a value?
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.
true === '4', meaning it is the default padding applied if you use <FlexItem padding/>
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.
But I also think, that this PR is not related to changing layout components, we can and should do this separately
reverted deleted file index.ts
...c/pages/autodiscover-sentinel/sentinel-databases-result/useSentinelDatabasesResultConfig.tsx
Show resolved
Hide resolved
...c/pages/autodiscover-sentinel/sentinel-databases-result/useSentinelDatabasesResultConfig.tsx
Outdated
Show resolved
Hide resolved
...c/pages/autodiscover-sentinel/sentinel-databases-result/useSentinelDatabasesResultConfig.tsx
Outdated
Show resolved
Hide resolved
...c/pages/autodiscover-sentinel/sentinel-databases-result/useSentinelDatabasesResultConfig.tsx
Show resolved
Hide resolved
.../autodiscover-sentinel/sentinel-databases/components/SentinelDatabases/SentinelDatabases.tsx
Outdated
Show resolved
Hide resolved
| useEffect(() => { | ||
| setTitle('Redis Sentinel Primary Groups Added') | ||
| if (!masters.length) { |
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.
does the title need to change back on component unmount?
overall, quite weird mechanism we have here
…view databases when nothing is to be added
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 it's worth extracting some of the styled components to .styles.ts files, other than that LGTM
- Split column definitions into individual files for better organization - Updated ResultColumn to fix TypeScript error with onAddInstance type - Adjusted column properties (minSize to maxSize) for consistency - Moved handleBackAdding and handleViewDatabases to useCallback hooks - Updated imports and colFactory to use new component structure
- Extracted column definitions to separate files - Updated useSentinelDatabasesConfig.tsx with new structure
- Created Summary component for result feedback - Fixed ResultColumn sizing property to minSize
Masters:

Result:
