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

[statusbar] add misc statusbar tooltips #6770

Merged
merged 1 commit into from
Dec 20, 2019
Merged

Conversation

vince-fugnitto
Copy link
Member

What it does

  • added statusbar tooltip for select language mode
  • added statusbar tooltip for select encoding
  • added statusbar tooltip for select indentation
  • added statusbar tooltip for select end of line sequence
  • added statusbar tooltip for go to line (clicking the statusbar item now triggers the go to line command)`

How to test

  1. verify that each tooltip described above correctly displays
  2. verify that the go to line statusbar item now triggers the go to line command

Review checklist

Reminder for reviewers

Signed-off-by: Vincent Fugnitto [email protected]

- added statusbar tooltip for `select language mode`
- added statusbar tooltip for `select encoding`
- added statusbar tooltip for `select indentation`
- added statusbar tooltip for `select end of line sequence`
- added statusbar tooltip for `go to line` (clicking the statusbar item now triggers the `go to line` command)`

Signed-off-by: Vincent Fugnitto <[email protected]>
@vince-fugnitto vince-fugnitto added the statusbar issues related to the statusbar label Dec 18, 2019
@vince-fugnitto vince-fugnitto self-assigned this Dec 18, 2019
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.

Looks good
Comment: When I start in electron and there is already a file in the editor(settings.json) the status bar does not show the options to test, I need to open a new file or close + re-open a file to see the status bar options.

If possible, when the browser is full screen, The tooltip overlaps the status bar text, could it be possible to put the tooltip above the status bar in this case
Look at the code OK
Tested on Ubuntu only

@vince-fugnitto
Copy link
Member Author

Comment: When I start in electron and there is already a file in the editor (settings.json) the status bar does not show the options to test, I need to open a new file or close + re-open a file to see the status bar options.

It worked correctly for me, I see all available statusbar items even if an editor is currently opened on startup. Which statusbar item did you not see?

If possible, when the browser is full screen, The tooltip overlaps the status bar text, could it be possible to put the tooltip above the status bar in this case

The tooltip is controlled by the browser and operating system.
I don't think there is a way using the default title attribute to be able to control the behavior easily.
For reference, you can also try in VS Code.

@lmcbout
Copy link
Contributor

lmcbout commented Dec 18, 2019

Which statusbar item did you not see?

All of the items on the right side

@vince-fugnitto
Copy link
Member Author

Which statusbar item did you not see?

All of the items on the right side

As we verified together, the contributed contributed statusbar items in question are only added when the editor itself has selection. If an editor does not have selection on startup, the statusbar items are not present which in this case is the observed behavior. Perhaps we can be smarter about when these items are created but I feel as though it is out of the scope of this particular pull request.

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 it in Gitpod. Looks good. Thank you !

@elaihau
Copy link
Contributor

elaihau commented Dec 19, 2019

BTW, I don't see tooltip text for the status bar button for the problems & warinings - not sure how difficult it is to add the tooltip for it.

@vince-fugnitto
Copy link
Member Author

BTW, I don't see tooltip text for the status bar button for the problems & warinings - not sure how difficult it is to add the tooltip for it.

Thanks! I actually covered it later in #6771

@vince-fugnitto vince-fugnitto merged commit cab90e1 into master Dec 20, 2019
@vince-fugnitto vince-fugnitto deleted the vf/statusbar branch December 20, 2019 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
statusbar issues related to the statusbar
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants