Skip to content

Added frontend UI for Hover and Go To Definition#221

Closed
nangtrongvuon wants to merge 29 commits intoxi-editor:masterfrom
nangtrongvuon:hover-definition
Closed

Added frontend UI for Hover and Go To Definition#221
nangtrongvuon wants to merge 29 commits intoxi-editor:masterfrom
nangtrongvuon:hover-definition

Conversation

@nangtrongvuon
Copy link
Copy Markdown
Member

@nangtrongvuon nangtrongvuon commented Jul 13, 2018

This PR adds frontend UI for hover and goto definition, both of which are LSP features implemented in xi-editor #717.

Currently, the UI elements are styled similar to XCode's way of showing quick help and definition.

To use Hover, Option+Click on the symbol desired, or move the mouse to the symbol while holding Option. To use Definition, Control+Click on the symbol desired.

Some screenshots of these elements can be found below:

Hover

The hover screen does not use markdown yet. I still feel this is open to discussion.
screen shot 2018-07-13 at 09 24 47

Definition

Definition is styled to be very similar to XCode's own definition view. Rows are highlighted on mouse hover, although the actual goto command is not implemented yet.

definition-orange
definition-green

Feedback
@raphlinus @cmyr @betterclever @scholtzan @jansol

There is still a bit of work to do - such as cleaning up code here and there, or more documentation in general, though I think it is ready for some preliminary feedback.

Feedback, especially on how the elements should look, is very much appreciated. My personal opinion is the hover view could use a little more work to make it look nicer, and a Markdown library could be put in Xi, although I don't know how that will work, considering Xi's status as an open source Google project. That being said, I'm open to any and all suggestions.

This will make the hover not behave erratically when moving small distances and prevent cases where
the hover is shown at the last mouse moved location instead.
- The commit fixes the issue where hover views would be sized to their
previous result. Now the hover view is sized snugly to its content.
- This commit also reverts HoverViewController being repurposed to accomodate definition
support. Definition view will be managed in a different view
controller in another commit.
The definition view now highlight rows on hover, much like XCode's own
definition view.
The hover effect now looks similar to XCode's own definition hover - it takes the user's alternate highlight color.
@jansol
Copy link
Copy Markdown
Contributor

jansol commented Jul 13, 2018

What's going on with the ^ in the definition screenshots?

@nangtrongvuon
Copy link
Copy Markdown
Member Author

It's just part of the popover, the arrow's origin is where the cursor was. MacOS's screenshot doesn't capture the cursor, so it's not very clear.

@jansol
Copy link
Copy Markdown
Contributor

jansol commented Jul 13, 2018

I mean the border looks weird in the screenshots. Like the draw order is wrong? It looks right in the hover screenshot.

@jansol
Copy link
Copy Markdown
Contributor

jansol commented Jul 13, 2018

Oh and please align the point of the ^ with the baseline of the text if possible, instead of centering it on the line. That way it's still clear what it points to but it doesn't obscure the word needlessly.

EDIT: just noticed you said it's positioned on the cursor. I'd still move it to the baseline of the line the cursor is on.

@nangtrongvuon
Copy link
Copy Markdown
Member Author

The border seems to be a screenshot thing where the popover shadow doesn’t get rendered. I might take other screenshots that aren’t window shots.

Gotcha on the line thing.

@cmyr
Copy link
Copy Markdown
Member

cmyr commented Jul 13, 2018

Very cool that this is working!

I noticed that the hover view seems to clip the definition if the definition is too long. We should probably either resize the view in this case, or maybe allow scrolling? What do you think?

@cmyr
Copy link
Copy Markdown
Member

cmyr commented Jul 13, 2018

Also I see what I think @jansol was pointing out, which is that in the goto view, the popup view draws a shadow over top of the arrow of the callout?

@nangtrongvuon
Copy link
Copy Markdown
Member Author

I’ll have to check again, but I don’t think it looks like that normally? I took the screenshot as a window specific screenshot, so maybe that could be why.

@cmyr
Copy link
Copy Markdown
Member

cmyr commented Jul 13, 2018

I was testing manually when I saw this.

@nangtrongvuon
Copy link
Copy Markdown
Member Author

Got it, I’ll check that too.

This commit also does a little refactoring - namely, methods related to
HoverViewController and DefinitionViewController are now an extension for EditViewController.
@nangtrongvuon
Copy link
Copy Markdown
Member Author

nangtrongvuon commented Jul 16, 2018

Both of the aforementioned issues were fixed. You can see how the popover views look like now here.

nh ch p man hinh 2018-07-16 luc 18 52 00
nh ch p man hinh 2018-07-16 luc 18 53 01

This also fixes the jarring resize that happens when scrolling stops.
Many repetitive names that references hover or definition have been renamed, as having "hover" or "definition" seemed redundant if they are coming from their respective view controllers.
@nangtrongvuon
Copy link
Copy Markdown
Member Author

I'll close this PR, since this work will be split in two parts - Hover and Definition.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants