Add completion status to setup sections #3823
Conversation
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
| const tooltipContent = ( | ||
| <div className={styles.infoTooltip}> | ||
| {infoIcon} | ||
| {SectionDescription[sectionKey]} |
There was a problem hiding this comment.
Do we want to adjust the tooltip text to include information about a section status or is the icon enough? We could add a generic message like "This section is incomplete" 🤔
There was a problem hiding this comment.
The current text should be ok for now. If we want to revisit we should add very clear text for the expectation/usage of the section. E.g Configure the extension to start working with DVC. Under the current configuration, the extension cannot initialize. Please follow the steps laid out in the section in order to get up and running.
| @@ -17,6 +17,7 @@ $error-color: var(--vscode-errorForeground); | |||
| $meta-cell-color: var(--vscode-descriptionForeground); | |||
| $accent-color: var(--button-primary-background); | |||
| $accent-color-transparent: var(--accent-transparency-3); | |||
| $passed-color: var(--vscode-testing-iconPassed); | |||
There was a problem hiding this comment.
Is there a better variable for this color that I missed?
webview/src/setup/components/App.tsx
Outdated
| @@ -128,6 +134,7 @@ export const App: React.FC = () => { | |||
| title="Studio" | |||
| sectionCollapsed={sectionCollapsed} | |||
| setSectionCollapsed={setSectionCollapsed} | |||
| hasError={!cliCompatible} | |||
There was a problem hiding this comment.
Noticed that the studio section shows the "DVC Not Available" screen if DVC is not compatible but shows the setting screen if the project is not initialized. Is this expected?
There was a problem hiding this comment.
That is how I set it up. Although connecting to Studio is a secondary concern we can connect without a project. We cannot however check to see if Studio is connected if we don't have access to the CLI (need to run dvc config studio.token).
|
After a discussion with Matt, we're going to remove the white checkmark icon, replacing it with the red error icon. It will reduce states and make things more clear :) Should be a quick change but converting this pr to a draft since I won't be working on it till tomorrow morning! |
| const tooltipContent = ( | ||
| <div className={styles.infoTooltip}> | ||
| {infoIcon} | ||
| {SectionDescription[sectionKey]} |
| @@ -17,6 +17,7 @@ $error-color: var(--vscode-errorForeground); | |||
| $meta-cell-color: var(--vscode-descriptionForeground); | |||
| $accent-color: var(--button-primary-background); | |||
| $accent-color-transparent: var(--accent-transparency-3); | |||
| $passed-color: var(--vscode-testing-iconPassed); | |||
| hasData: true, | ||
| isPythonExtensionUsed: true, | ||
| isStudioConnected: true, | ||
| needsGitCommit: false, |
There was a problem hiding this comment.
This is only a suggestion, but by having anything that is set to false or undefined be optional in renderApp that would make it easier to see what is it that a test is actually testing with and for. It'd make writing new test much easier as well.
There was a problem hiding this comment.
I was thinking about this as well since that's how we write other test files when it comes to a lot of props. Will update renderApp and adjust the rest of the tests in a followup.
webview/src/shared/components/sectionContainer/styles.module.scss
Outdated
Show resolved
Hide resolved
|
Code Climate has analyzed commit 16a228b and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (85% is the threshold). This pull request will bring the total coverage in the repository to 94.8%. View more on Code Climate. |
Demo
https://user-images.githubusercontent.com/43496356/236336529-94e7ab49-cb57-4218-a384-a43ded399a36.movtrimmed.mov
Part of #3434