Add experiment state indicators to commit records in table #3591

mattseddon · 2023-03-30T11:12:39Z

1/3 #3585 <- this <- #3595 <- #3596

Look a "new" feature.

This PR moves the experiment indicators (count) from experiment rows up to commit records. Previously we only did this for checkpoints. That no longer makes sense.

I also fixed a bug with the top-level plotted indicator. Hidden rows were being incorrectly removed from the count.

Demo

Screen.Recording.2023-03-31.at.10.06.58.am.mov

qlty-cloud-legacy · 2023-03-30T11:20:12Z

Code Climate has analyzed commit e5f2fe6 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.9% (-0.1% change).

View more on Code Climate.

mattseddon · 2023-03-30T23:11:52Z

extension/src/experiments/webview/contract.ts

  hasValidDvcYaml: boolean
  isShowingMoreCommits: boolean
  rows: Commit[]
+  selectedForPlotsCount: number


[F] Easier/more consistent to pass this from the extension.

mattseddon · 2023-03-30T23:13:07Z

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

    })
  })

+  describe('Sub-rows middle states indicators', () => {


[F] These were deleted previously. I have adapted the old tests to work on the commit record.

mattseddon · 2023-03-30T23:14:02Z

webview/src/experiments/util/rows.ts

-  row: Row<Experiment>
-): number => selectedForPlotsCount + (row.original?.selected ? 1 : 0)
-
-export const getSelectedForPlotsCount = (


[F] I cannot explain why I did this... maybe responsiveness... 😕

mattseddon · 2023-03-30T23:14:30Z

webview/src/stories/Table.stories.tsx

+    '1ba7bcd'
+  ])
+}
+WithMiddleStates.play = async ({ canvasElement }) => {


[F] Again this story was deleted but now I've brought it back

mattseddon · 2023-03-30T23:15:29Z

webview/src/test/sort.ts

    }
  ],
+  selectedForPlotsCount: 0,
  sorts: []


[Q] Why is this file even here???? Shouldn't it be with the fixtures 😕

At a guess, it was put here since this file is only used by the webview. The extension tests don't seem to use it. I'm all for moving it though!

mattseddon · 2023-03-30T23:25:53Z

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

      )
    })

+    it('should show not change the plotted indicator when plotted experiments are hidden', () => {


[F] This is a new test that would have caught the bug.

mattseddon · 2023-03-30T23:26:26Z

webview/src/experiments/components/table/CellRowActions.tsx

-        className={cx(styles.rowActions, hidden && styles.hidden)}
-        data-testid={testId}
-      >
+      <div className={cx(styles.rowActions)} data-testid={testId}>


[F] I tried to clean up the classes and props but this needs a good clean out.

You can remove the cx() here since there is only one class.

mattseddon · 2023-03-30T23:26:50Z

webview/src/experiments/components/table/Indicators.tsx

 }) => {
  const content = (
    <button
-      className={cx(styles.indicatorIcon, count && styles.indicatorWithCount)}


indicatorWithCount also no longer existed

Same here, you can remove cx()

mattseddon · 2023-03-30T23:27:17Z

webview/src/experiments/components/table/styles.module.scss

          justify-content: center;
          width: 1.3rem;
-          height: 100%;
+          height: 2rem;


[F] Indicator was in the middle of the parent icon

mattseddon · 2023-03-30T23:27:30Z

webview/src/experiments/components/table/styles.module.scss

      }
-      .experimentGroup.expandedGroup & {
-        display: none;
-      }


sroy3 · 2023-03-31T12:58:32Z

webview/src/experiments/components/table/CellRowActions.tsx

-        className={cx(styles.rowActions, hidden && styles.hidden)}
-        data-testid={testId}
-      >
+      <div className={cx(styles.rowActions)} data-testid={testId}>


You can remove the cx() here since there is only one class.

sroy3 · 2023-03-31T12:59:02Z

webview/src/experiments/components/table/Indicators.tsx

 }) => {
  const content = (
    <button
-      className={cx(styles.indicatorIcon, count && styles.indicatorWithCount)}


Same here, you can remove cx()

julieg18

Great work!

* Remove experiment checkpoints from table * Remove experiment checkpoints from tree (#3587) * Remove experiment checkpoints from filters (#3588) * Remove experiment checkpoints from select experiments to plot (#3589) * Remove experiment checkpoints from plot color collection (#3590) * Add experiment state counts to commit record (#3591) * Remove references to experiment checkpoints from walkthrough (#3595) * Remove experiment checkpoints from table context menu (#3596) * Remove checkpoints from custom plots (#3610)

mattseddon added the product PR that affects product label Mar 30, 2023

mattseddon self-assigned this Mar 30, 2023

mattseddon changed the base branch from main to remove-checkpoints-from-plot-color March 30, 2023 21:03

mattseddon changed the base branch from remove-checkpoints-from-plot-color to main March 30, 2023 21:06

mattseddon changed the base branch from main to remove-checkpoints-from-plot-color March 30, 2023 21:07

mattseddon force-pushed the remove-checkpoints-from-plot-color branch from 8209f05 to 3bb053a Compare March 30, 2023 22:34

Base automatically changed from remove-checkpoints-from-plot-color to remove-checkpoints-from-webview March 30, 2023 22:34

mattseddon force-pushed the add-experiment-middle-states branch 2 times, most recently from 405ec0e to cad9a8f Compare March 30, 2023 23:09

mattseddon changed the title ~~Add experiment state counts to commit records in table~~ Add experiment state indicators to commit records in table Mar 30, 2023

mattseddon commented Mar 30, 2023

View reviewed changes

mattseddon force-pushed the add-experiment-middle-states branch from cad9a8f to abe3cb3 Compare March 30, 2023 23:25

mattseddon commented Mar 30, 2023

View reviewed changes

mattseddon marked this pull request as ready for review March 30, 2023 23:27

mattseddon requested review from julieg18 and sroy3 as code owners March 30, 2023 23:27

This was referenced Mar 31, 2023

Remove references to experiment checkpoints from walkthrough #3595

Merged

Remove experiment checkpoints from table context menu #3596

Merged

Clean up Experiments Table SCSS #3597

Closed

sroy3 approved these changes Mar 31, 2023

View reviewed changes

julieg18 approved these changes Mar 31, 2023

View reviewed changes

Add experiment state counts to commit record

a9da396

mattseddon force-pushed the add-experiment-middle-states branch from abe3cb3 to a9da396 Compare April 1, 2023 05:47

mattseddon merged commit db7a1c9 into remove-checkpoints-from-webview Apr 1, 2023

mattseddon deleted the add-experiment-middle-states branch April 1, 2023 07:13

mattseddon mentioned this pull request Apr 3, 2023

Remove checkpoints from custom plots #3610

Merged

Add experiment state indicators to commit records in table #3591

Add experiment state indicators to commit records in table #3591

Uh oh!

Conversation

mattseddon commented Mar 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1/3 #3585 <- this <- #3595 <- #3596

Demo

Uh oh!

qlty-cloud-legacy bot commented Mar 30, 2023

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!

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!

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!

julieg18 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mattseddon commented Mar 30, 2023 •

edited

Loading