Add toggle to show only changed columns in experiments table #4402

mattseddon · 2023-08-02T02:44:36Z

Revisited whilst looking at #4229

Closes #1994

This PR gives users the ability to switch the experiments table into a show-only changed columns mode. The behaviour closely follows the --only-changed flag for exp show (documented here):

Demo

Screen.Recording.2023-08-02.at.12.36.23.pm.mov

extension/src/experiments/columns/collect/index.ts

mattseddon · 2023-08-02T04:36:38Z

extension/src/experiments/webview/messages.ts

  private getWebviewData(): TableData {
+    const filters = this.experiments.getFilters()


[I] Follow up by making all of this async

mattseddon · 2023-08-02T08:59:10Z

webview/src/experiments/components/emptyState/AddColumns.tsx

+    <div>
+      <p>No Columns Selected.</p>
+      <StartButton onClick={selectColumns} text="Add Columns" />
+      {showOnlyChanged && (


[F] Good thing I remembered this because without it users would have been stuck.

Screen.Recording.2023-08-02.at.6.59.20.pm.mov

julieg18

Great work!

julieg18 · 2023-08-02T14:37:45Z

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

@@ -210,6 +210,14 @@ $badge-size: 0.85rem;
  }
 }

+.onlyChanged {
+  background-color: $indicator-badge-background;


Minor, the hover color seems a little weird for the selected icon. I expected a lighter or darker shade, but instead you see a completely different color in most cases.

Maybe we could borrow a primary button's color and hover color for the "checked" only-changed icon 🤔

PTAL at the update. We I can followup if needed.

julieg18 · 2023-08-02T14:39:34Z

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

+        tooltipContent="Toggle Show Only Changed Columns"
+        delay={[1000, 0]}
+      >
+        <button


Is there a reason why we're basically adding a new feature instead of building off of selected columns? Aka, "Show Only Changed" selects only changed columns and deselects none-changed columns?

There's also a possibility of users being confused on why "selected" columns just aren't showing up in the table and assume something is broken 🤔

Is there a reason why we're basically adding a new feature instead of building off of selected columns? Aka, "Show Only Changed" selects only changed columns and deselects none-changed columns?

Here is a list of things to consider with the approach of building on top of selecting columns

User takes a long time setting up mono-repo only to display the columns related to the project they are working in. Toggles show only changed - all selection is lost.

What happens when show only changed is toggled off? Would we hold the previous state and return to that? Does the selection stay the same when show only changed is turned off?

What happens when selection has changed when only changed is turned on? After applying only changed I deselect a number of columns. What happens when the data updates? Do these columns reappear? Can other new changed columns appear/get selected?

Filtering can update the columns selected for show only changed. Would this also change the selection?

There's also a possibility of users being confused on why "selected" columns just aren't showing up in the table and assume something is broken 🤔

This is why I tried to make the indicator as obvious as possible.

so tl;dr - it gets complicated fast. We can discuss tomorrow when we catch up.

julieg18 · 2023-08-02T14:43:08Z

webview/src/experiments/components/emptyState/AddColumns.tsx

+          appearance="secondary"
+          isNested={true}
+          onClick={toggleShowOnlyChanged}
+          text="Show Only Changed"


Minor! The "X" does make it more clear but when I saw this I assumed that only changed was off since the text is "Show Only Changed". Maybe "Disable Only Changed", Turn Off Only Changed" or "Toggle Only Changed"?

Updated to "Disable".

codeclimate · 2023-08-02T22:30:15Z

Code Climate has analyzed commit 6ba80a4 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.

BradyJ27 · 2023-08-07T19:29:16Z

Thanks for this! @mattseddon One small thing I noticed was when using the "show only changed" setting, the "columns" tab on the left no longer works to filter out columns. I wasn't sure if this was a limitation or a bug.

mattseddon · 2023-08-07T22:04:10Z

@BradyJ27 show only changed is an overlay on top of the selected columns. It automatically hides unchanged columns that are selected but does not change the selected state of the columns. De-selecting columns that are still visible is possible:

Screen.Recording.2023-08-08.at.7.55.46.am.mov

See #4402 (comment) above for more details on why this implementation was chosen.

BradyJ27 · 2023-08-08T01:53:35Z

@BradyJ27 show only changed is an overlay on top of the selected columns. It automatically hides unchanged columns that are selected but does not change the selected state of the columns. De-selecting columns that are still visible is possible:

Screen.Recording.2023-08-08.at.7.55.46.am.mov
See #4402 (comment) above for more details on why this implementation was chosen.

Sure, this makes sense. The columns tab will still show all columns, but selecting/de-selecting hidden columns does nothing when the "toggle only changed" is on. Thanks for clarifying!

mattseddon added the product PR that affects product label Aug 2, 2023

mattseddon self-assigned this Aug 2, 2023

mattseddon commented Aug 2, 2023

View reviewed changes

extension/src/experiments/columns/collect/index.ts Show resolved Hide resolved

mattseddon mentioned this pull request Aug 2, 2023

exp show: --json does not respect the --only-changed flag iterative/dvc#9544

Closed

codeclimate bot reviewed Aug 2, 2023

View reviewed changes

extension/src/experiments/columns/collect/index.ts Outdated Show resolved Hide resolved

mattseddon commented Aug 2, 2023

View reviewed changes

mattseddon marked this pull request as ready for review August 2, 2023 05:23

mattseddon requested review from sroy3 and julieg18 as code owners August 2, 2023 05:23

mattseddon requested a review from shcheklein August 2, 2023 05:23

mattseddon force-pushed the show-only-changed branch from a94fcbd to 90acad8 Compare August 2, 2023 08:58

mattseddon commented Aug 2, 2023

View reviewed changes

julieg18 approved these changes Aug 2, 2023

View reviewed changes

mattseddon added 9 commits August 3, 2023 08:21

add show only changed columns to experiments table

aa56ba4

persist show only changed between sessions

af5bf46

add experiment webview tests

49d64a9

add experiment webview tests

a37c549

add unit tests for changed column collection

c567c15

refactor collect columns with changed values

2e7387a

remove .only from integration tests

ffe8bc4

add remove show only changed button to get started screen

ff8e2c8

address review comments

6ba80a4

mattseddon force-pushed the show-only-changed branch from 90acad8 to 6ba80a4 Compare August 2, 2023 22:21

mattseddon enabled auto-merge (squash) August 2, 2023 22:25

mattseddon merged commit d672c12 into main Aug 2, 2023

mattseddon deleted the show-only-changed branch August 2, 2023 22:37

mattseddon mentioned this pull request Aug 3, 2023

Collect webview messages async #4405

Merged

mattseddon mentioned this pull request Aug 3, 2023

Only show relevant columns in experiments table #1994

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add toggle to show only changed columns in experiments table #4402

Add toggle to show only changed columns in experiments table #4402

mattseddon commented Aug 2, 2023 •

edited

Loading

mattseddon Aug 2, 2023

mattseddon Aug 2, 2023

mattseddon Aug 2, 2023

julieg18 left a comment

julieg18 Aug 2, 2023

mattseddon Aug 2, 2023 •

edited

Loading

julieg18 Aug 2, 2023

julieg18 Aug 2, 2023

mattseddon Aug 2, 2023 •

edited

Loading

mattseddon Aug 2, 2023

julieg18 Aug 2, 2023

mattseddon Aug 2, 2023

codeclimate bot commented Aug 2, 2023

BradyJ27 commented Aug 7, 2023

mattseddon commented Aug 7, 2023

BradyJ27 commented Aug 8, 2023

		private getWebviewData(): TableData {
		const filters = this.experiments.getFilters()

Add toggle to show only changed columns in experiments table #4402

Add toggle to show only changed columns in experiments table #4402

Conversation

mattseddon commented Aug 2, 2023 • edited Loading

Demo

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

julieg18 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattseddon Aug 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattseddon Aug 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codeclimate bot commented Aug 2, 2023

BradyJ27 commented Aug 7, 2023

mattseddon commented Aug 7, 2023

BradyJ27 commented Aug 8, 2023

mattseddon commented Aug 2, 2023 •

edited

Loading

mattseddon Aug 2, 2023 •

edited

Loading

mattseddon Aug 2, 2023 •

edited

Loading