Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions public/apps/account/tenant-switch-panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,9 @@ export function TenantSwitchPanel(props: TenantSwitchPanelProps) {
}
};

const invalidCustomTenant =
tenantSwitchRadioIdSelected === CUSTOM_TENANT_RADIO_ID && !selectedCustomTenantOption[0];

let content;

if (isMultiTenancyEnabled) {
Expand All @@ -249,6 +252,7 @@ export function TenantSwitchPanel(props: TenantSwitchPanelProps) {
In current EUI if put into the child of radio option, clicking in the combo box will not
show the drop down list since the radio option consumes the click event. */}
<EuiComboBox
placeholder="Select a custom tenant"
options={customTenantOptions}
singleSelection={{ asPlainText: true }}
selectedOptions={selectedCustomTenantOption}
Expand Down Expand Up @@ -297,8 +301,8 @@ export function TenantSwitchPanel(props: TenantSwitchPanelProps) {

<EuiButton
data-test-subj="confirm"
fill
disabled={!isMultiTenancyEnabled}
fill={isMultiTenancyEnabled && !invalidCustomTenant}
Copy link
Member

Choose a reason for hiding this comment

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

I see we've now added a condition on fill here. I just want to confirm if this is intended behavior.

fill is related to whether the bg of the button is filled in or transparent. See https://eui.elastic.co/pr_3350/#/navigation/button

Screen Shot 2022-10-03 at 10 31 57 AM

Copy link
Collaborator Author

@RyanL1997 RyanL1997 Oct 3, 2022

Choose a reason for hiding this comment

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

Hi @cwperks, thanks for the review! I talked to @shanilpa, and he suggested me to change the disabled button to the ghosted disabled without the fill option, because that's the pattern we are moving forward with. That's why I added the condition to the fill option.

Copy link

Choose a reason for hiding this comment

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

@RyanL1997 that's correct. We're looking to reduce some of the visual clutter by making use of the ghosted disabled state. Thanks for the review @cwperks.

disabled={!isMultiTenancyEnabled || invalidCustomTenant}
onClick={handleTenantConfirmation}
>
Confirm
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ exports[`Account menu -tenant switch panel confirm button and renders renders wh
},
]
}
placeholder="Select a custom tenant"
selectedOptions={Array []}
singleSelection={
Object {
Expand Down Expand Up @@ -190,6 +191,7 @@ exports[`Account menu -tenant switch panel confirm button and renders renders wh
},
]
}
placeholder="Select a custom tenant"
selectedOptions={Array []}
singleSelection={
Object {
Expand Down Expand Up @@ -304,6 +306,7 @@ exports[`Account menu -tenant switch panel confirm button and renders renders wh
},
]
}
placeholder="Select a custom tenant"
selectedOptions={Array []}
singleSelection={
Object {
Expand Down Expand Up @@ -418,6 +421,7 @@ exports[`Account menu -tenant switch panel confirm button and renders renders wh
},
]
}
placeholder="Select a custom tenant"
selectedOptions={Array []}
singleSelection={
Object {
Expand Down Expand Up @@ -532,6 +536,7 @@ exports[`Account menu -tenant switch panel confirm button and renders renders wh
},
]
}
placeholder="Select a custom tenant"
selectedOptions={Array []}
singleSelection={
Object {
Expand Down