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

Update CSS for description elements of tree nodes #9742

Merged
merged 1 commit into from
Jul 21, 2021

Conversation

msujew
Copy link
Member

@msujew msujew commented Jul 15, 2021

What it does

Closes #9741
Closes #7189

Aligns our tree node CSS with vscode (using opacity and default foreground coloring on description elements). Allows for better readability in light mode.

Problem markers on current master. Note the hard to read light theme:

image

image

With the change in place:

image

image

Compared to vscode:

image

image

How to test

Problem markers:

  1. Open a project with problem markers in Theia.
  2. Focus/Select the problem marker. Assess readability, look and feel.
  3. Change between the different themes (dark/light/solarized, etc.) and repeat step 2.

Search view:

  1. Search for any text (that exists within the project)
  2. Focus/Select the file node in the tree.
  3. Readability on the file path should be improved for the light theme.

Debug view:

  1. Create a breakpoint
  2. Focus/Select the breakpoint in the debug view.
  3. Readability on the breakpoint path should be improved for the light theme.

Review checklist

Reminder for reviewers

@vince-fugnitto vince-fugnitto added problems issues related to the problems widget ui/ux issues related to user interface / user experience labels Jul 15, 2021
@vince-fugnitto
Copy link
Member

@msujew I'll take a look at the changes 👍 In the past we tried to fix the problem, but it was true for multiple trees:

@msujew msujew force-pushed the marker-css-alignment branch from 8382323 to 6eec607 Compare July 15, 2021 12:29
@msujew
Copy link
Member Author

msujew commented Jul 15, 2021

@vince-fugnitto Thanks for the input. I applied the same fix to the other trees as well.

@msujew msujew changed the title Improve problem marker readability in light mode Improve tree node readability in light mode Jul 15, 2021
@vince-fugnitto vince-fugnitto added search in workspace issues related to the search-in-workspace debug issues that related to debug functionality labels Jul 15, 2021
@vince-fugnitto
Copy link
Member

I was hoping there could be a general solution in @theia/core for all trees, and possibly trees in the future but since this is low-effort enough I'd be fine to go with this solution for the moment 👍

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

In a multi-root workspace, we should also think of handling marker info nodes:

image

@msujew msujew force-pushed the marker-css-alignment branch from 6eec607 to 175f7b1 Compare July 15, 2021 13:09
@msujew
Copy link
Member Author

msujew commented Jul 15, 2021

In a multi-root workspace, we should also think of handling marker info nodes

Forgot about those, done.

I was hoping there could be a general solution in @theia/core for all trees

What would you propose? We would basically need some kind of common css class for all marker info elements, right? I could try to implement a more general solution instead of the current approach.

@vince-fugnitto
Copy link
Member

I was hoping there could be a general solution in @theia/core for all trees

What would you propose? We would basically need some kind of common class for all marker info elements, right? I could try to implement a more general solution instead of the current approach.

Perhaps it is out of scope for the moment (I'd be fine with this approach and a refactoring after). I was thinking to have a generic class for node descriptions which can be handled in core and refactor some trees not to include their custom styling:

export const TREE_CLASS = 'theia-Tree';
export const TREE_CONTAINER_CLASS = 'theia-TreeContainer';
export const TREE_NODE_CLASS = 'theia-TreeNode';
export const TREE_NODE_CONTENT_CLASS = 'theia-TreeNodeContent';
export const TREE_NODE_TAIL_CLASS = 'theia-TreeNodeTail';
export const TREE_NODE_SEGMENT_CLASS = 'theia-TreeNodeSegment';
export const TREE_NODE_SEGMENT_GROW_CLASS = 'theia-TreeNodeSegmentGrow';
export const EXPANDABLE_TREE_NODE_CLASS = 'theia-ExpandableTreeNode';
export const COMPOSITE_TREE_NODE_CLASS = 'theia-CompositeTreeNode';
export const TREE_NODE_CAPTION_CLASS = 'theia-TreeNodeCaption';
export const TREE_NODE_INDENT_GUIDE_CLASS = 'theia-tree-node-indent';

@msujew msujew force-pushed the marker-css-alignment branch from 175f7b1 to 99d612c Compare July 15, 2021 14:00
@msujew
Copy link
Member Author

msujew commented Jul 15, 2021

I'd be fine with this approach and a refactoring after

I'd usually agree, but one of my meetings got spontaneously canceled and I got an hour of free time ;)

I was thinking to have a generic class for node descriptions

Yeah, that's what I thought about as well. I implemented it that way, WDYT?

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

The changes look much better to me 👍 and work correctly as well!
Do you mind updating the commit message and description to reflect the changes better, I believe the scope changed since the initial implementation.

@msujew msujew force-pushed the marker-css-alignment branch from 99d612c to 94a8081 Compare July 15, 2021 14:32
@msujew msujew changed the title Improve tree node readability in light mode Update CSS for description elements of tree nodes Jul 15, 2021
@msujew msujew force-pushed the marker-css-alignment branch from 94a8081 to 9fe8303 Compare July 15, 2021 20:13
Adds a new `theia-TreeNodeInfo` CSS class that should be used by every part of a treenode that adds additional info. The info part is displayed with a lower opacity.
Examples for this CSS class can be seen for the `owner` and `position` info of problem markers and the `path` of a debugging breakpoint.
@msujew msujew force-pushed the marker-css-alignment branch from 9fe8303 to 25a2919 Compare July 17, 2021 10:23
@msujew
Copy link
Member Author

msujew commented Jul 17, 2021

@vince-fugnitto FYI, I updated the severity marker as well, so that it aligns with vscode.

image

@vince-fugnitto
Copy link
Member

vince-fugnitto commented Jul 19, 2021

@vince-fugnitto FYI, I updated the severity marker as well, so that it aligns with vscode.

Looks great! In the future we can think of supporting #9744.

@vince-fugnitto vince-fugnitto merged commit 9578b84 into eclipse-theia:master Jul 21, 2021
@vince-fugnitto vince-fugnitto added this to the 1.16.0 milestone Jul 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debug issues that related to debug functionality problems issues related to the problems widget search in workspace issues related to the search-in-workspace ui/ux issues related to user interface / user experience
Projects
None yet
2 participants