Skip to content

Conversation

@Raubzeug
Copy link
Contributor

@Raubzeug Raubzeug commented Sep 9, 2024

closes #1252

Stand

CI Results

Test Status: ⚠️ FLAKY

📊 Full Report

Total Passed Failed Flaky Skipped
124 119 0 5 0

Bundle Size: ✅

Current: 78.86 MB | Main: 78.86 MB
Diff: +0.00 MB (0.00%)

✅ Bundle size unchanged.

ℹ️ CI Information
  • Test recordings for failed tests are available in the full report.
  • Bundle size is measured for the entire 'dist' directory.
  • 📊 indicates links to detailed reports.
  • 🔺 indicates increase, 🔽 decrease, and ✅ no change in bundle size.

artemmufazalov
artemmufazalov previously approved these changes Sep 11, 2024

const normalizedClusterName = name ?? clusterName;

const {currentData: baseClusterInfo} = clusterApi.useGetClusterInfoQuery(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this query contain info form getClusterBaseInfo? Why do use both?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, you're right. Will use it from getClusterBaseInfo if available!

artemmufazalov
artemmufazalov previously approved these changes Sep 12, 2024
setDefaultClusterTab(state, action: PayloadAction<ClusterTab>) {
state.defaultClusterTab = action.payload;
},
setClusterTitle: (state, action: PayloadAction<string>) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add only selector, something like this:

const createClusterInfoSelector = createSelector(
    (clusterName?: string) => clusterName,
    (clusterName) => clusterApi.endpoints.getClusterInfo.select(clusterName),
);

export const selectClusterInfo = createSelector(
    (state: RootState) => state,
    (_state: RootState, clusterName?: string) => createClusterInfoSelector(clusterName),
    (state, selectGetTopic) => selectGetTopic(state).data,
);

export const selectClusterTitle = createSelector(
    (_state: RootState, clusterName?: string) => clusterName,
    (state: RootState, clusterName?: string) => selectClusterInfo(state, clusterName),
    (clusterName, clusterInfo) => {
        const {Name, Domain} = clusterInfo?.clusterData || {};

        return Name || clusterName || normalizeDomain(Domain) || CLUSTER_DEFAULT_TITLE;
    },
);

}: ClusterProps) {
const container = React.useRef<HTMLDivElement>(null);

const clusterTitle = useTypedSelector(selectClusterTitle);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const clusterTitle = useTypedSelector(selectClusterTitle);
const clusterTitle = useTypedSelector((state) => selectClusterTitle(state, clusterName ?? undefined));

@Raubzeug Raubzeug requested a review from ValeraS September 13, 2024 09:08
ValeraS
ValeraS previously approved these changes Sep 13, 2024
export const selectClusterInfo = createSelector(
(state: RootState) => state,
(_state: RootState, clusterName?: string) => createClusterInfoSelector(clusterName),
(state, selectGetTopic) => selectGetTopic(state).data,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

selectGetTopic - please rename

@ValeraS ValeraS dismissed their stale review September 13, 2024 09:28

selectGetTopic - please rename

@Raubzeug Raubzeug requested a review from ValeraS September 13, 2024 10:28
@Raubzeug Raubzeug merged commit 125d556 into main Sep 13, 2024
@Raubzeug Raubzeug deleted the 1252-use-domain branch September 13, 2024 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

use Domain name instead of Cluster name when cluster name is not available

4 participants