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.

Sign up for GitHub

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

Jump to bottom

Match tooltip styles to VS Code #2353

Merged
merged 7 commits into from
Sep 8, 2022
Merged

Match tooltip styles to VS Code #2353

merged 7 commits into from
Sep 8, 2022

Conversation

mattseddon
Copy link
Member

@mattseddon mattseddon commented Sep 8, 2022 •
edited
Loading

Closes #2254 by matching tooltip styling to VS Code's native trees.

Demo

Screen.Recording.2022-09-08.at.1.31.25.pm.mov

Screen Shot 2022-09-08 at 1 32 46 pm

  • add a border to tooltips
  • remove arrows from tooltips and context menus => Remove the triangle (on top) from the context menus from Review: table styles #2241
  • remove unnecessary code and styles

@mattseddon mattseddon self-assigned this Sep 8, 2022
@mattseddon mattseddon force-pushed the add-border-to-tooltips branch from d29e216 to e33f819 Compare September 8, 2022 02:41
@mattseddon mattseddon changed the title Add borders to all tooltips Match tooltip styles to VS Code Sep 8, 2022
@mattseddon mattseddon added product PR that affects product and removed 🏠 housekeeping labels Sep 8, 2022
@mattseddon
Copy link
Member Author

@julieg18 looks like the truncation tooltip for experiment names isn't working anymore.

@mattseddon mattseddon marked this pull request as ready for review September 8, 2022 03:56
@julieg18
Copy link
Contributor

julieg18 commented Sep 8, 2022

@julieg18 looks like the truncation tooltip for experiment names isn't working anymore.

I'm not sure what you mean 🤔 I've never seen a tooltip appear for experiment names (that I can remember 😅):

image

What is the behavior supposed to be for experiment names?

julieg18
julieg18 approved these changes Sep 8, 2022
View reviewed changes
Copy link
Contributor

@julieg18 julieg18 left a comment

Choose a reason for hiding this comment

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

Great work!

webview/src/shared/components/tooltip/styles.module.scss
.tooltipContent {
border: 1px solid $tooltip-border;
white-space: pre-wrap;
padding: 5px;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we decrease the plots info tooltips padding now that they already get some padding from .tooltipContent:

image

Copy link
Member Author

Choose a reason for hiding this comment

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

I changed the padding here.

@mattseddon mattseddon enabled auto-merge (squash) September 8, 2022 19:51
@mattseddon mattseddon disabled auto-merge September 8, 2022 19:52
@codeclimate
Copy link

codeclimate bot commented Sep 8, 2022

Code Climate has analyzed commit 74e3b43 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 96.9% (0.0% change).

View more on Code Climate.

@mattseddon mattseddon merged commit ff41c01 into main Sep 8, 2022
@mattseddon mattseddon deleted the add-border-to-tooltips branch September 8, 2022 20:39
@mattseddon
Copy link
Member Author

@julieg18 looks like the truncation tooltip for experiment names isn't working anymore.

I'm not sure what you mean 🤔 I've never seen a tooltip appear for experiment names (that I can remember 😅):

image

What is the behavior supposed to be for experiment names?

I remember that are some point the branch name (maybe) would show a tooltip when it was truncated... might have dreamed it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@julieg18 julieg18 julieg18 approved these changes

@sroy3 sroy3 sroy3 approved these changes

@shcheklein shcheklein shcheklein approved these changes

@wolmir wolmir wolmir approved these changes

@maxagin maxagin Awaiting requested review from maxagin

Assignees

@mattseddon mattseddon

Labels
product PR that affects product
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Data trees: handle better broken DVC files
5 participants
@mattseddon @julieg18 @wolmir @shcheklein @sroy3