Skip to content

Commit

Permalink
Feature/project wizard next button validation (#2773)
Browse files Browse the repository at this point in the history
* add missing `localize` calls and jsdoc

* style and disable the ok button if step incomplete

- also fixes the missing border on the Browse... button for the labeledFolderInput component

* ensure the selected interpreter is a valid option in the dropdown list

Note that the conda dropdown items are hardcoded, so none of the options are valid. Once the conda interpreters are attributed to actual runtimes, we will be able to validate the selected item in the dropdown list.

* fix selected interpreter logic

First, try to use the existing selection. If it's not an option in the dropdown, then try to use the preferred interpreter. If that's still not an option, then there isn't a default selection.
  • Loading branch information
sharon-wang authored Apr 15, 2024
1 parent 2b22378 commit 4289bc3
Show file tree
Hide file tree
Showing 9 changed files with 121 additions and 40 deletions.
3 changes: 3 additions & 0 deletions build/lib/stylelint/vscode-known-variables.json
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,9 @@
"--vscode-positronModalDialog-buttonActiveBackground",
"--vscode-positronModalDialog-buttonBackground",
"--vscode-positronModalDialog-buttonBorder",
"--vscode-positronModalDialog-buttonDisabledBackground",
"--vscode-positronModalDialog-buttonDisabledBorder",
"--vscode-positronModalDialog-buttonDisabledForeground",
"--vscode-positronModalDialog-buttonForeground",
"--vscode-positronModalDialog-buttonHoverBackground",
"--vscode-positronModalDialog-checkboxBackground",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
border-radius: 4px;
color: var(--vscode-positronModalDialog-buttonForeground);
background: var(--vscode-positronModalDialog-buttonBackground);

border: 1px solid var(--vscode-positronModalDialog-buttonBorder);
}

.positron-modal-dialog-box
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,12 @@
align-items: end;
justify-content: space-between;
}

.positron-modal-dialog-box
.ok-cancel-action-bar
.action-bar-button:disabled {
color: var(--vscode-positronModalDialog-buttonDisabledForeground);
background-color: var(--vscode-positronModalDialog-buttonDisabledBackground);
border: 1px solid var(--vscode-positronModalDialog-buttonDisabledBorder);
cursor: default;
}
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,10 @@ export const ProjectNameLocationStep = (props: PropsWithChildren<NewProjectWizar
'Set project name and location'
))()}
cancelButtonConfig={{ onClick: props.cancel }}
nextButtonConfig={{ onClick: nextStep }}
nextButtonConfig={{
onClick: nextStep,
disable: !projectConfig.projectName || !projectConfig.parentFolder
}}
backButtonConfig={{ onClick: props.back }}
>
<PositronWizardSubStep
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ export const ProjectTypeStep = (props: PropsWithChildren<NewProjectWizardStepPro
onClick: props.cancel
}}
nextButtonConfig={{
onClick: nextStep
onClick: nextStep,
disable: !selectedProjectType
}}
/>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ import { DisposableStore } from 'vs/base/common/lifecycle';
import { getEnvTypeEntries, getPythonInterpreterEntries, locationForNewEnv } from 'vs/workbench/browser/positronNewProjectWizard/utilities/pythonEnvironmentStepUtils';
import { PositronWizardStep } from 'vs/workbench/browser/positronNewProjectWizard/components/wizardStep';
import { PositronWizardSubStep } from 'vs/workbench/browser/positronNewProjectWizard/components/wizardSubStep';
import { DropDownListBox } from 'vs/workbench/browser/positronComponents/dropDownListBox/dropDownListBox';
import { DropDownListBox, DropDownListBoxEntry } from 'vs/workbench/browser/positronComponents/dropDownListBox/dropDownListBox';
import { RadioButtonItem } from 'vs/workbench/browser/positronComponents/positronModalDialog/components/radioButton';
import { RadioGroup } from 'vs/workbench/browser/positronComponents/positronModalDialog/components/radioGroup';
import { EnvironmentSetupType, LanguageIds, PythonEnvironmentType } from 'vs/workbench/browser/positronNewProjectWizard/interfaces/newProjectWizardEnums';
import { InterpreterEntry } from 'vs/workbench/browser/positronNewProjectWizard/components/steps/pythonInterpreterEntry';
import { DropdownEntry } from 'vs/workbench/browser/positronNewProjectWizard/components/steps/dropdownEntry';
import { getSelectedInterpreter } from 'vs/workbench/browser/positronNewProjectWizard/utilities/interpreterDropDownUtils';
import { InterpreterInfo, getSelectedInterpreter } from 'vs/workbench/browser/positronNewProjectWizard/utilities/interpreterDropDownUtils';

/**
* The PythonEnvironmentStep component is specific to Python projects in the new project wizard.
Expand All @@ -47,13 +47,6 @@ export const PythonEnvironmentStep = (props: PropsWithChildren<NewProjectWizardS
const [envType, setEnvType] = useState(
projectConfig.pythonEnvType ?? PythonEnvironmentType.Venv
);
const [selectedInterpreter, setSelectedInterpreter] = useState(
getSelectedInterpreter(
projectConfig.selectedRuntime,
runtimeStartupService,
LanguageIds.Python
)
);
const [interpreterEntries, setInterpreterEntries] =
useState(
// It's possible that the runtime discovery phase is not complete, so we need to check
Expand All @@ -67,6 +60,14 @@ export const PythonEnvironmentStep = (props: PropsWithChildren<NewProjectWizardS
envType
)
);
const [selectedInterpreter, setSelectedInterpreter] = useState(
getSelectedInterpreter(
projectConfig.selectedRuntime,
interpreterEntries,
runtimeStartupService,
LanguageIds.Python
)
);

const envTypeEntries = getEnvTypeEntries();

Expand All @@ -88,9 +89,10 @@ export const PythonEnvironmentStep = (props: PropsWithChildren<NewProjectWizardS
];

// Utils
const getInterpreter = () => {
const getInterpreter = (entries: DropDownListBoxEntry<string, InterpreterInfo>[]) => {
return getSelectedInterpreter(
selectedInterpreter,
entries,
runtimeStartupService,
LanguageIds.Python
);
Expand All @@ -103,15 +105,14 @@ export const PythonEnvironmentStep = (props: PropsWithChildren<NewProjectWizardS
setEnvSetupType(pythonEnvSetupType);
// If the user selects an existing environment, update the interpreter entries dropdown
// to show the unfiltered list of all existing interpreters.
setInterpreterEntries(
getPythonInterpreterEntries(
runtimeStartupService,
languageRuntimeService,
pythonEnvSetupType,
envType
)
const entries = getPythonInterpreterEntries(
runtimeStartupService,
languageRuntimeService,
pythonEnvSetupType,
envType
);
const selectedRuntime = getInterpreter();
setInterpreterEntries(entries);
const selectedRuntime = getInterpreter(entries);
setSelectedInterpreter(selectedRuntime);
setProjectConfig({ ...projectConfig, pythonEnvSetupType, selectedRuntime });
};
Expand All @@ -120,15 +121,14 @@ export const PythonEnvironmentStep = (props: PropsWithChildren<NewProjectWizardS
// on the selected environment type, and the project configuration is updated as well.
const onEnvTypeSelected = (pythonEnvType: PythonEnvironmentType) => {
setEnvType(pythonEnvType);
setInterpreterEntries(
getPythonInterpreterEntries(
runtimeStartupService,
languageRuntimeService,
envSetupType,
pythonEnvType
)
const entries = getPythonInterpreterEntries(
runtimeStartupService,
languageRuntimeService,
envSetupType,
pythonEnvType
);
const selectedRuntime = getInterpreter();
setInterpreterEntries(entries);
const selectedRuntime = getInterpreter(entries);
setSelectedInterpreter(selectedRuntime);
setProjectConfig({ ...projectConfig, pythonEnvType, selectedRuntime });
};
Expand Down Expand Up @@ -167,7 +167,7 @@ export const PythonEnvironmentStep = (props: PropsWithChildren<NewProjectWizardS
envType
);
setInterpreterEntries(entries);
const selectedRuntime = getInterpreter();
const selectedRuntime = getInterpreter(entries);
setSelectedInterpreter(selectedRuntime);
setProjectConfig({
...projectConfig,
Expand Down Expand Up @@ -198,7 +198,8 @@ export const PythonEnvironmentStep = (props: PropsWithChildren<NewProjectWizardS
title: (() => localize(
'positronNewProjectWizard.createButtonTitle',
"Create"
))()
))(),
disable: !selectedInterpreter
}}
>
<PositronWizardSubStep
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,6 @@ export const RConfigurationStep = (props: PropsWithChildren<NewProjectWizardStep

// Hooks to manage the startup phase and interpreter entries.
const [startupPhase, setStartupPhase] = useState(runtimeStartupService.startupPhase);
const [selectedInterpreter, setSelectedInterpreter] = useState(
getSelectedInterpreter(projectConfig.selectedRuntime, runtimeStartupService, LanguageIds.R)
);
const [interpreterEntries, setInterpreterEntries] =
useState(
// It's possible that the runtime discovery phase is not complete, so we need to check
Expand All @@ -50,7 +47,14 @@ export const RConfigurationStep = (props: PropsWithChildren<NewProjectWizardStep
[] :
getRInterpreterEntries(runtimeStartupService, languageRuntimeService)
);

const [selectedInterpreter, setSelectedInterpreter] = useState(
getSelectedInterpreter(
projectConfig.selectedRuntime,
interpreterEntries,
runtimeStartupService,
LanguageIds.R
)
);
// Handler for when the interpreter is selected. The project configuration is updated with the
// selected interpreter.
const onInterpreterSelected = (identifier: string) => {
Expand Down Expand Up @@ -87,6 +91,7 @@ export const RConfigurationStep = (props: PropsWithChildren<NewProjectWizardStep
// Set the selected interpreter to the preferred interpreter if it is available.
const selectedRuntime = getSelectedInterpreter(
selectedInterpreter,
entries,
runtimeStartupService,
LanguageIds.R
);
Expand Down Expand Up @@ -115,7 +120,8 @@ export const RConfigurationStep = (props: PropsWithChildren<NewProjectWizardStep
title: (() => localize(
'positronNewProjectWizard.createButtonTitle',
"Create"
))()
))(),
disable: !selectedInterpreter
}}
>
<PositronWizardSubStep
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,19 +92,53 @@ export const getPreferredRuntime = (runtimeStartupService: IRuntimeStartupServic
};

/**
* Retrieves the selected interpreter for the given languageId.
* Determines if the interpreter is in the dropdown list.
* @param interpreter The interpreter to check.
* @param interpreterEntries The interpreter entries.
* @returns True if the interpreter is in the dropdown list, false otherwise.
*/
const isInterpreterInDropdown = (
interpreter: ILanguageRuntimeMetadata | undefined,
interpreterEntries: DropDownListBoxEntry<string, InterpreterInfo>[]
) => {
if (!interpreter || !interpreterEntries.length) {
return false;
}
return interpreterEntries.find(entry => {
if (entry instanceof DropDownListBoxItem) {
return entry.options.identifier === interpreter.runtimeId;
}
return false;
});
};

/**
* Retrieves the selected interpreter for the given languageId if it is a valid option in the
* dropdown list.
* @param existingSelection The existing selection.
* @param interpreterEntries The interpreter entries.
* @param runtimeStartupService The runtime startup service.
* @param languageId The languageId of the runtime to retrieve.
* @returns The already selected interpreter if it matches the languageId, the preferred interpreter
* if it exists, or undefined if no preferred interpreter is found.
*/
export const getSelectedInterpreter = (
existingSelection: ILanguageRuntimeMetadata | undefined,
interpreterEntries: DropDownListBoxEntry<string, InterpreterInfo>[],
runtimeStartupService: IRuntimeStartupService,
languageId: LanguageIds
) => {
return existingSelection?.languageId === languageId ?
existingSelection :
getPreferredRuntime(runtimeStartupService, languageId);
// Return the existing selection if it is in the dropdown list.
if (isInterpreterInDropdown(existingSelection, interpreterEntries)) {
return existingSelection;
}

// Return the preferred interpreter if it is in the dropdown list.
const preferredInterpreter = getPreferredRuntime(runtimeStartupService, languageId);
if (isInterpreterInDropdown(preferredInterpreter, interpreterEntries)) {
return preferredInterpreter;
}

// Otherwise, there doesn't appear to be a valid selection.
return undefined;
};
24 changes: 24 additions & 0 deletions src/vs/workbench/common/theme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1528,6 +1528,14 @@ export const POSITRON_MODAL_DIALOG_BUTTON_ACTIVE_BACKGROUND = registerColor('pos
hcLight: darken(POSITRON_MODAL_DIALOG_BUTTON_BACKGROUND, 0.15)
}, localize('positronModalDialog.buttonActiveBackground', "Positron modal dialog button active background color."));

// Positron modal dialog button disabled background color.
export const POSITRON_MODAL_DIALOG_BUTTON_DISABLED_BACKGROUND = registerColor('positronModalDialog.buttonDisabledBackground', {
dark: null,
light: null,
hcDark: null,
hcLight: null
}, localize('positronModalDialog.buttonDisabledBackground', "Positron modal dialog button disabled background color."));

// Positron modal dialog button foreground color.
export const POSITRON_MODAL_DIALOG_BUTTON_FOREGROUND = registerColor('positronModalDialog.buttonForeground', {
dark: buttonSecondaryForeground,
Expand All @@ -1544,6 +1552,14 @@ export const POSITRON_MODAL_DIALOG_DEFAULT_BUTTON_FOREGROUND = registerColor('po
hcLight: buttonForeground
}, localize('positronModalDialog.defaultButtonForeground', "Positron modal dialog default button foreground color."));

// Positron modal dialog button disabled foreground color.
export const POSITRON_MODAL_DIALOG_BUTTON_DISABLED_FOREGROUND = registerColor('positronModalDialog.buttonDisabledForeground', {
dark: disabledForeground,
light: disabledForeground,
hcDark: disabledForeground,
hcLight: disabledForeground
}, localize('positronModalDialog.buttonDisabledForeground', "Positron modal dialog button disabled foreground color."));

// Positron modal dialog button border color.
export const POSITRON_MODAL_DIALOG_BUTTON_BORDER = registerColor('positronModalDialog.buttonBorder', {
dark: null,
Expand All @@ -1552,6 +1568,14 @@ export const POSITRON_MODAL_DIALOG_BUTTON_BORDER = registerColor('positronModalD
hcLight: contrastBorder
}, localize('positronModalDialog.buttonBorder', "Positron modal dialog button border color."));

// Positron modal dialog button disabled border color.
export const POSITRON_MODAL_DIALOG_BUTTON_DISABLED_BORDER = registerColor('positronModalDialog.buttonDisabledBorder', {
dark: disabledForeground,
light: disabledForeground,
hcDark: disabledForeground,
hcLight: disabledForeground
}, localize('positronModalDialog.buttonDisabledBorder', "Positron modal dialog button disabled border color."));

// < --- Positron Modal Dialog Text Input --- >

// Positron modal dialog text input background color.
Expand Down

0 comments on commit 4289bc3

Please sign in to comment.