Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust plot ribbon tooltips #4135

Merged
merged 8 commits into from
Jun 20, 2023
Merged

Adjust plot ribbon tooltips #4135

merged 8 commits into from
Jun 20, 2023

Conversation

julieg18
Copy link
Contributor

@julieg18 julieg18 commented Jun 17, 2023

  • removes Created column
  • adds table description
  • adds info icon to ribbon blocks

Demo

https://github.com/iterative/vscode-dvc/assets/43496356/f2c24368-d01f-4064-a18b-1c6c4c41f201

Screen.Recording.2023-06-20.at.10.54.01.AM.mov
Screenshot 2023-06-20 at 10 56 09 AM

Fixes #3942

* remove Created column
* add table description
@julieg18 julieg18 added the product PR that affects product label Jun 17, 2023
@julieg18 julieg18 self-assigned this Jun 17, 2023
@julieg18 julieg18 marked this pull request as ready for review June 19, 2023 22:51
@julieg18 julieg18 requested a review from shcheklein June 19, 2023 22:56
Copy link
Member

@mattseddon mattseddon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good iteration. Some notes: I wouldn't assume that we won't need to deal with dates now. I'd like to see what the tooltip looks like if we include the first 3 params and the first 3 metrics.

extension/src/experiments/model/util.ts Outdated Show resolved Hide resolved
@@ -42,6 +42,9 @@ export const RibbonBlockTooltip: React.FC<{
))}
</tbody>
</table>
<p className={styles.tooltipTableDescription}>
*Reflects the first three columns (minus Created) in the table
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*Reflects the first three columns (minus Created) in the table

  • Could swap * with another info icon.
  • Change minus to excluding.
  • Need to mention that it is the experiments table.
  • Give instructions on how to update. Something like "Drag and drop columns to the start of the table to update.

This could also go after the commit information.

I'm almost tempted to suggest turning the ribbon into a mini version of the table that always displays the selected columns but I think that is a story in itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to mention that it is the experiments table.
Change minus to excluding.

👍

Give instructions on how to update. Something like "Drag and drop columns to the start of the table to update.

I considered that but didn't want to make the message too verbose since it's already rather lengthy... I'll post an image and see what others think though.

Could swap * with another info icon.

Good idea, I'll try it and see how it looks!

This could also go after the commit information.

Why would we want to move the table below the commit information? I figured that the table information is more important than the commit hence why it's first 🤔

Copy link
Contributor Author

@julieg18 julieg18 Jun 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's what it looks like with extra instructions:

Screenshot 2023-06-20 at 11 00 04 AM

What do we think? Plus, do we need additional instructions or is it obvious enough that a user can move columns once they start to work with the table?

Though I suppose if we increase the table rows, the extra text length would look better 🤔

@julieg18 julieg18 enabled auto-merge (squash) June 20, 2023 17:38
@codeclimate
Copy link

codeclimate bot commented Jun 20, 2023

Code Climate has analyzed commit 56e2144 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.

@julieg18 julieg18 merged commit 906fed0 into main Jun 20, 2023
@julieg18 julieg18 deleted the adjust-plot-ribbon-tooltips branch June 20, 2023 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product PR that affects product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plots: make it easier to identify exp info
2 participants