Point users to Python extension in DVC Setup #4124
Conversation
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
| @@ -187,6 +186,28 @@ describe('App', () => { | |||
| }) | |||
| }) | |||
|
|
|||
| it('should show "Set Env" and "Install (pip)" button tooltips when dvc is unavailable and Python extension is not installed', () => { | |||
There was a problem hiding this comment.
My original plan was to add tests for checking if buttons and tooltips were disabled depending on circumstances but for some reason, tooltips and buttons weren't disabling in the tests. They are working in Storybook and in development but not in testing. Any ideas what's going on?
There was a problem hiding this comment.
the magic virtual dom probably doesn't set the properly correctly
| }> = ({ dataTestId, disabled, children }) => ( | ||
| <Tooltip | ||
| content={ | ||
| <span data-testid={dataTestId}> |
There was a problem hiding this comment.
Not sure if we need the extra tooltips since it does tell you to install the python extension in the text 🤔
There was a problem hiding this comment.
I don't think we need the tooltip either. It looks very strange with almost the same text popping up next to the original.
| }> = ({ dataTestId, disabled, children }) => ( | ||
| <Tooltip | ||
| content={ | ||
| <span data-testid={dataTestId}> |
There was a problem hiding this comment.
I don't think we need the tooltip either. It looks very strange with almost the same text popping up next to the original.
| @@ -2,14 +2,13 @@ import React from 'react' | |||
| import styles from './styles.module.scss' | |||
| import { Icon } from '../../../shared/components/Icon' | |||
| import { Extensions } from '../../../shared/components/icons' | |||
| import { ExtensionLink } from '../shared/ExtensionLink' | |||
There was a problem hiding this comment.
[Q] Why not move the text out from the original paragraph, put it under the buttons and use this component?
I think that would
- draw attention to the text
- remove the need to show a tooltip.
There was a problem hiding this comment.
Good idea! Here's what that looks like so far:
I adjusted the paragraph styling so that it looks more connected to the disabled buttons vs a separate paragraph entirely.
|
Code Climate has analyzed commit b92216c 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 95.2% (0.0% change). View more on Code Climate. |
Demo
https://github.com/iterative/vscode-dvc/assets/43496356/aec5ac6e-d39b-4799-aa52-3468a0fd8e03Screen.Recording.2023-06-19.at.11.01.20.AM.mov
Part of #3935