Add setup warning about latest tested version #3895

julieg18 · 2023-05-15T15:45:26Z

Demo

Screen.Recording.2023-05-17.at.3.57.26.PM.mov

Part of #3434

webview/src/setup/components/App.tsx

julieg18 · 2023-05-16T13:35:52Z

webview/src/setup/components/App.test.tsx

          version: '1.0.0'
        },
        hasData: false,
+        isAboveLatestTestedVersion: false,


As mentioned by Stephanie a couple prs ago, we need to stop our tests from writing SetupData from scratch for every test. In #3434 and will work on this in our next sprint!

I have also run into this. My changes will conflict. I can do this today.

julieg18 · 2023-05-16T13:37:23Z

webview/src/setup/components/App.test.tsx

        shareLiveToStudio: false
      })

-      const iconWrapper = screen.getAllByTestId('info-tooltip-toggle')[0]


Noticed array selection was being used in info icon tests which is bad practice since the setup order will eventually change.

Yep, I have run into this as well

webview/src/setup/components/App.tsx

julieg18 · 2023-05-16T13:59:47Z

webview/src/setup/components/App.tsx

        title="DVC"
-        isSetup={isDvcSetup}
+        icon={getDvcStatusIcon(isDvcSetup, !!isAboveLatestTestedVersion)}
+        secondaryTooltipText={


Message is pretty lengthy. Maybe we could simplify it or remove the summary text about the DVC section 🤔

We could easily remove "Warning" from the text as the icon already states that there is a warning present. Also, there should be a comma before "which".

Message is pretty lengthy. Maybe we could simplify it or remove the summary text about the DVC section

Yes and yes. However, I think we need to update a bit. The desired behaviour is if the user is getting unexpected behaviour then they should first try to upgrade the extension and if that is not possible downgrade the CLI. Here is a rough attempt at improving the message.

The located version has not been tested against the extension. If you are experiencing unexpected behaviour first try to update the extension. If there are no updates available then please downgrade DVC to the same minor version as displayed.

That can be polished but it is a start. Make sense?

Makes sense! Here's what I've got so far:

webview/src/setup/components/App.tsx

sroy3 · 2023-05-16T16:20:03Z

webview/src/setup/components/App.tsx

        title="DVC"
-        isSetup={isDvcSetup}
+        icon={getDvcStatusIcon(isDvcSetup, !!isAboveLatestTestedVersion)}
+        secondaryTooltipText={


We could easily remove "Warning" from the text as the icon already states that there is a warning present. Also, there should be a comma before "which".

webview/src/shared/components/sectionContainer/InfoTooltip.tsx

mattseddon · 2023-05-16T22:32:50Z

webview/src/shared/components/sectionContainer/InfoTooltip.tsx

+    icon?: TooltipIconType
+  }>
+> = ({ icon = TooltipIconType.INFO, sectionKey, children }) => {
  const infoIcon = (


[N] I think leaving this named infoIcon is misleading because it is no longer always the information icon. Maybe indicatorIcon would be a better name.

Good idea! We actually use the term info in multiple places regarding the component. Data id, component name, class, etc. I'll rename the different places in a followup :)

mattseddon · 2023-05-16T22:37:16Z

webview/src/setup/components/App.tsx

        title="DVC"
-        isSetup={isDvcSetup}
+        icon={getDvcStatusIcon(isDvcSetup, !!isAboveLatestTestedVersion)}
+        secondaryTooltipText={


Message is pretty lengthy. Maybe we could simplify it or remove the summary text about the DVC section

Yes and yes. However, I think we need to update a bit. The desired behaviour is if the user is getting unexpected behaviour then they should first try to upgrade the extension and if that is not possible downgrade the CLI. Here is a rough attempt at improving the message.

The located version has not been tested against the extension. If you are experiencing unexpected behaviour first try to update the extension. If there are no updates available then please downgrade DVC to the same minor version as displayed.

That can be polished but it is a start. Make sense?

mattseddon · 2023-05-16T22:39:30Z

webview/src/setup/components/App.test.tsx

        shareLiveToStudio: false
      })

-      const iconWrapper = screen.getAllByTestId('info-tooltip-toggle')[0]


Yep, I have run into this as well

mattseddon · 2023-05-16T22:40:20Z

webview/src/setup/components/App.test.tsx

          version: '1.0.0'
        },
        hasData: false,
+        isAboveLatestTestedVersion: false,


I have also run into this. My changes will conflict. I can do this today.

qlty-cloud-legacy · 2023-05-18T13:47:51Z

Code Climate has analyzed commit b9368ed and detected 3 issues on this pull request.

Here's the issue category breakdown:

Category	Count
Duplication	3

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.0% (0.0% change).

View more on Code Climate.

julieg18 added 3 commits May 15, 2023 10:25

Allow secondary text in SecondaryDescription

e3f1dc1

Merge branch 'main' into add-dvc-setup-latest-tested-version-warning

99b37f1

Merge branch 'main' into add-dvc-setup-latest-tested-version-warning

088a2db

julieg18 added the product PR that affects product label May 15, 2023

julieg18 self-assigned this May 15, 2023

julieg18 mentioned this pull request May 15, 2023

Fix broken setup stories #3896

Merged

qlty-cloud-legacy bot reviewed May 15, 2023

View reviewed changes

webview/src/setup/components/App.tsx Show resolved Hide resolved

webview/src/setup/components/App.tsx Show resolved Hide resolved

julieg18 added 5 commits May 15, 2023 11:58

Add backend logic

2f9dc78

Add tests

5791c6e

Fix vscode tests

355889d

Merge branch 'main' into add-dvc-setup-latest-tested-version-warning

5eddc5e

Message change

4b3bff3

julieg18 commented May 16, 2023

View reviewed changes

julieg18 marked this pull request as ready for review May 16, 2023 14:02

julieg18 requested review from mattseddon and sroy3 as code owners May 16, 2023 14:02

julieg18 requested a review from shcheklein May 16, 2023 14:02

sroy3 approved these changes May 16, 2023

View reviewed changes

julieg18 added 2 commits May 16, 2023 12:02

Improve message

8eda49e

Refactor InfoTooltip icon creation

5f42eae

mattseddon approved these changes May 16, 2023

View reviewed changes

mattseddon mentioned this pull request May 16, 2023

Add default test data to setup app tests #3908

Merged

julieg18 and others added 6 commits May 17, 2023 15:17

infoIcon to indicatorIcon

bfd347a

Adjust tooltip text

dd826dc

Merge branch 'main' into add-dvc-setup-latest-tested-version-warning

6c017e8

Merge branch 'main' into add-dvc-setup-latest-tested-version-warning

d82d9c0

Fix change caused by merge

c36ed6e

Merge branch 'main' into add-dvc-setup-latest-tested-version-warning

b9368ed

julieg18 enabled auto-merge (squash) May 18, 2023 13:41

julieg18 merged commit 30935d8 into main May 18, 2023

julieg18 deleted the add-dvc-setup-latest-tested-version-warning branch May 18, 2023 14:17

Add setup warning about latest tested version #3895

Add setup warning about latest tested version #3895

Uh oh!

Conversation

julieg18 commented May 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Demo

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

julieg18 May 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qlty-cloud-legacy bot commented May 18, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

julieg18 commented May 15, 2023 •

edited

Loading

julieg18 May 17, 2023 •

edited

Loading