-
Notifications
You must be signed in to change notification settings - Fork 98
Respect the EC multi-node license option #5257
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
Respect the EC multi-node license option #5257
Conversation
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.
Looks great Salah! 🔥 Just a few suggestions but non blocking!
@@ -472,6 +472,14 @@ const AppLicenseComponent = () => { | |||
enabled{" "} | |||
</span> | |||
) : null} | |||
{isEmbeddedCluster && | |||
!appLicense?.isEmbeddedClusterMultinodeDisabled ? ( |
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.
we can leave out the ternary and use logical &&
operator here for improved readability
const getStepNumber = (stepId: string) => { | ||
return getSidebarSteps().findIndex((s) => s.id === stepId); | ||
}; | ||
|
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.
optional but we can refactor this so we can separate presentation logic with step rendering and prevent multiple calls to getStepProps
interface StepProps {
step: {
id: string;
title: string;
children?: React.ReactNode;
};
stepProps: {
icon: string;
fontClass: string;
textColor: string;
};
}
const SidebarStep = ({ step, stepProps }: StepProps) => {
const { icon, fontClass, textColor } = stepProps;
return (
<div
key={step.id}
className="tw-p-8 tw-shadow-[0_1px_0_#c4c8ca] tw-font-medium"
>
<div className="tw-flex">
<Icon
icon={icon}
size={16}
className="tw-mr-2"
/>
<span
className={`${fontClass} ${textColor} tw-flex-1`}
>
{step.title}
</span>
</div>
{step.children}
</div>
);
};
</span> | ||
</div> | ||
|
||
{getSidebarSteps().map((step, index) => ( |
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.
{steps.map((step, index) => (
<SidebarStep
key={step.id}
step={step}
stepProps={getStepProps(index)}
/>
))}
|
||
const GetStartedSidebar = () => { | ||
const { adminConsoleMetadata, appsList } = state; | ||
|
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.
const steps = getSidebarSteps();
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.
Go code looks fine, I'll leave the JS to Mia
What this PR does / why we need it:
Adds the ability to limit Embedded Cluster installations to a single node.
Which issue(s) this PR fixes:
SC-121720
Does this PR require a test?
Yes
Does this PR require a release note?
Does this PR require documentation?
Single node:
Multi node:
License field icon: