Skip to content

Commit

Permalink
Merge pull request #21 from mturley/jira-rhoaieng-10833-followup-1
Browse files Browse the repository at this point in the history
Properly reset single select option only when ready
  • Loading branch information
DaoDaoNoCode authored Nov 1, 2024
2 parents c1d57bf + aeac1c8 commit f2fe81b
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 19 deletions.
10 changes: 6 additions & 4 deletions frontend/src/components/SimpleSelect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export type SimpleGroupSelectOption = {

type SimpleSelectProps = {
options?: SimpleSelectOption[];
isLoadingOptions?: boolean;
groupedOptions?: SimpleGroupSelectOption[];
value?: string;
toggleLabel?: React.ReactNode;
Expand All @@ -56,6 +57,7 @@ const SimpleSelect: React.FC<SimpleSelectProps> = ({
isDisabled,
onChange,
options,
isLoadingOptions = false,
groupedOptions,
placeholder = 'Select...',
value,
Expand Down Expand Up @@ -88,14 +90,14 @@ const SimpleSelect: React.FC<SimpleSelectProps> = ({
);

// If there is only one option, call the onChange function
const totalOptionsKey = totalOptions.length === 1 ? totalOptions[0].key : null;
React.useEffect(() => {
if (totalOptionsKey) {
onChange(totalOptionsKey, false);
const singleOptionKey = totalOptions.length === 1 ? totalOptions[0].key : null;
if (singleOptionKey && !isLoadingOptions) {
onChange(singleOptionKey, false);
}
// We don't want the callback function to be a dependency
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [totalOptionsKey]);
}, [totalOptions, isLoadingOptions]);

return (
<>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,13 @@ type InferenceServiceFrameworkSectionProps = {
setData: UpdateObjectAtPropAndValue<CreatingInferenceServiceObject>;
modelContext?: SupportedModelFormats[];
registeredModelFormat?: string;
selectedRuntimeName?: string;
};

const InferenceServiceFrameworkSection: React.FC<InferenceServiceFrameworkSectionProps> = ({
data,
setData,
modelContext,
registeredModelFormat,
selectedRuntimeName,
}) => {
const [modelsContextLoaded, loaded, loadError] = useModelFramework(
modelContext ? undefined : data.servingRuntimeName,
Expand Down Expand Up @@ -62,23 +60,19 @@ const InferenceServiceFrameworkSection: React.FC<InferenceServiceFrameworkSectio
? `${framework.name} - ${framework.version}`
: `${framework.name}`;
return {
// SimpleSelect component will only trigger onChange
// when there is only one option and has a different key
// We need to make the key unique to make sure it can
// be auto-selected when different serving runtimes have the same one option
key: `${selectedRuntimeName} - ${name}`,
key: name,
label: name,
};
})}
isLoadingOptions={!modelContext && !loaded}
isFullWidth
toggleLabel={
dataFormatVersion
? `${dataFormatName} - ${dataFormatVersion}`
: dataFormatName || placeholderText
}
onChange={(option) => {
// De-constructing and omit selected serving runtime name
const [, name, version] = option.split(' - ');
const [name, version] = option.split(' - ');
setData('format', { name, version });
}}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,6 @@ const ManageInferenceServiceModal: React.FC<ManageInferenceServiceModalProps> =
setData={setCreateData}
modelContext={projectContext?.currentServingRuntime?.spec.supportedModelFormats}
registeredModelFormat={registeredModelDeployInfo?.modelFormat}
selectedRuntimeName={createData.servingRuntimeName}
/>
</StackItem>
<StackItem>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,28 @@ const useModelFramework = (
namespace?: string,
): [models: SupportedModelFormats[], loaded: boolean, loadError: Error | undefined] => {
const [models, setModels] = React.useState<SupportedModelFormats[]>([]);
const [loaded, setLoaded] = React.useState(false);
const [loadedFrameworksForRuntimeName, setLoadedFrameworksForRuntimeName] = React.useState<
string | null
>(null);
const [loadError, setLoadError] = React.useState<Error | undefined>(undefined);

React.useEffect(() => {
if (!name || !namespace) {
return;
}
setLoadError(undefined);
setLoaded(false);
setLoadedFrameworksForRuntimeName(null);
getServingRuntime(name, namespace)
.then((servingRuntime) => {
setModels(servingRuntime.spec.supportedModelFormats || []);
setLoaded(true);
setLoadedFrameworksForRuntimeName(name);
})
.catch((e) => {
setLoadError(e);
});
}, [name, namespace]);

return [models, loaded, loadError];
return [models, loadedFrameworksForRuntimeName === name, loadError];
};

export default useModelFramework;
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,6 @@ const ManageKServeModal: React.FC<ManageKServeModalProps> = ({
setData={setCreateDataInferenceService}
modelContext={servingRuntimeSelected?.spec.supportedModelFormats}
registeredModelFormat={registeredModelDeployInfo?.modelFormat}
selectedRuntimeName={servingRuntimeSelected?.metadata.name}
/>
<KServeAutoscalerReplicaSection
data={createDataInferenceService}
Expand Down

0 comments on commit f2fe81b

Please sign in to comment.