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

markers: update format of markers #7344

Merged
merged 1 commit into from
Mar 18, 2020
Merged

markers: update format of markers #7344

merged 1 commit into from
Mar 18, 2020

Conversation

vince-fugnitto
Copy link
Member

What it does

This pull-request updates the formatting of the markers present in the problems-view to be consistent with the latest updates present from VS Code. Namely, the pull-request updates the markers so that:

  1. the severity icon is displayed first.
  2. the message is displayed secondly (the message has been bumped since it holds the most important information).
  3. the owner/source is then displayed with an attached code if available.
  4. the line and column number is then displayed.
Before After
image image

How to test

  1. start the application and open the problems-view.
  2. ensure that the view has markers (preferably from different sources), and verify the new formatting.

Review checklist

Reminder for reviewers

Signed-off-by: vince-fugnitto vincent.fugnitto@ericsson.com

@vince-fugnitto
markers: update format of markers
e25609a 
This commit updates the formatting of markers present in the `problems-view`
to be consistent with VS Code. Namely, the commit enforces that the marker
message is the most important data, that the `owner` or `source` is displayed with the
`code` if present, and the `line number` and `column number` are displayed last.

Signed-off-by: vince-fugnitto <vincent.fugnitto@ericsson.com>
@vince-fugnitto vince-fugnitto added markers issues related to problem markers ui/ux issues related to user interface / user experience labels Mar 16, 2020
@vince-fugnitto vince-fugnitto self-assigned this Mar 16, 2020
@vince-fugnitto vince-fugnitto marked this pull request as ready for review March 16, 2020 19:36
elaihau
elaihau approved these changes Mar 17, 2020
View reviewed changes
Copy link
Contributor

@elaihau elaihau left a comment

Choose a reason for hiding this comment

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

I tested on my local with Chrome browser in the following scenarios

  • single-root and multi root workspace
  • problem markers added by language servers
  • problem markers created by customized tasks

worked as expected.

lmcbout
lmcbout approved these changes Mar 17, 2020
View reviewed changes
Copy link
Contributor

@lmcbout lmcbout left a comment

Choose a reason for hiding this comment

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

Works as expected
Tested with Chrome on Ubuntu 18.04
Note: the icon in front of the file is more visible in VSode than in Theia. For example the "JAVA" icon in Theia is the "JAVA" cup icon, but the dark color on a DARK theme is not as visible as the "JAVA" icon they use in VSCODE . Same for other icons "TS, JS..."

@vince-fugnitto
Copy link
Member Author

Note: the icon in front of the file is more visible in VSode than in Theia. For example the "JAVA" icon in Theia is the "JAVA" cup icon, but the dark color on a DARK theme is not as visible as the "JAVA" icon they use in VSCODE . Same for other icons "TS, JS..."

@lmcbout we are using different default icon themes. If you'd like to test, set the icon theme to seti which is vscode's default theme.

@lmcbout
Copy link
Contributor

lmcbout commented Mar 17, 2020

@lmcbout we are using different default icon themes. If you'd like to test, set the icon theme to seti which is vscode's default theme.

@vince-fugnitto I think the icons we use are not as visible as the one we use if we set the icons as VSCOde icons. It still works :)

@vince-fugnitto
Copy link
Member Author

I'll merge tomorrow if there are no objections.

Anasshahidd21
Anasshahidd21 approved these changes Mar 18, 2020
View reviewed changes
Copy link
Contributor

@Anasshahidd21 Anasshahidd21 left a comment

Choose a reason for hiding this comment

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

Works fine on my side as well.

@vince-fugnitto vince-fugnitto merged commit d4d607b into master Mar 18, 2020
@vince-fugnitto vince-fugnitto deleted the vf/problem-marker-formatting branch March 18, 2020 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lmcbout lmcbout lmcbout approved these changes

@elaihau elaihau elaihau approved these changes

@Anasshahidd21 Anasshahidd21 Anasshahidd21 approved these changes

Assignees

@vince-fugnitto vince-fugnitto

Labels
markers issues related to problem markers ui/ux issues related to user interface / user experience
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vince-fugnitto @lmcbout @elaihau @Anasshahidd21