-
Notifications
You must be signed in to change notification settings - Fork 452
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
Revamp server status bar item. #3478
Revamp server status bar item. #3478
Conversation
24766b6
to
f0f3d82
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, looks good to me. The status is a lot more visible and access to opening logs / clearing workspace should hopefully fix some issues.
Only other consideration is whether users might be annoyed if they're doing something not related to Java and yet the status is a lot more prominent now. A user mentioned this but I don't think there's much we can do if the project itself is a Java project (particularly now that we have a global status bar).
@@ -32,7 +32,11 @@ export namespace serverStatus { | |||
|
|||
export function initialize() { | |||
serverTasks.onDidUpdateServerTask(tasks => { | |||
isBusy = tasks.some(task => !(task.complete)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. I just read that some
doesn't short-circuit. find
is an improvement 👍
What about the icons, if no background color change, do we need to change the icon to BTW, @rgrunber, I changed the icon from 👍 to ☕, in this PR, but I'm not sure if that |
If no proper icon for java warning, we can still coffee icon since the text expresses its meaning as well. |
I think it should be fine to use the coffee codicon. What about using text to represent status vs. codicons ("Ready", "Warning", "Error", 👍 , Does " ☕ Java: 👍 " look weird ? Benefit would be saving a bit of space at the bottom. " ☕ : 👍 " seems it could be confusing. |
I prefer |
Signed-off-by: Sheng Chen <[email protected]>
+1, icon+text is better than two icons. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
To me, the status shortcut list is a good way to expose features.
One follow-up request is to enable 3rd party extensions to contribute to the status shortcut list. For instance, vscode-java-pack offers many webviews to set up Java extension. If these options could appear in the status shortcut quickpick, users would find them more easily.
Just be aware we may encounter similar kinds of problems that we did trying to merge eclipse-jdtls/eclipse.jdt.ls#2821 . Opening more things to the progress reporting means if they don't end their progress reporting the bar will "spin" forever. |
This PR made following changes to the server status bar item.
Move to the left. This aligns VS Code's status bar UX guideline: Place primary (global) items on the left.
Display the progress more explicitly
Use background color to highlight the status item when action is required
Make the status bar item as an entrance to access other useful commands.