Bugzilla – Full Text Bug Listing
| Summary: | [Hover] JSDoc hovers are clipped and show scrollbar unnecessarily | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [ECD] Orion | Reporter: | Eric Moffatt <emoffatt> | ||||
| Component: | Client | Assignee: | Eric Moffatt <emoffatt> | ||||
| Status: | RESOLVED FIXED | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | curtis.windatt.public, Michael_Rennie, Silenio_Quarti | ||||
| Version: | unspecified | ||||||
| Target Milestone: | 9.0 | ||||||
| Hardware: | PC | ||||||
| OS: | Windows 7 | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
OK, here's a fix fir 1) the height issue as well as a related issue 2) when you have folded comment sections. Fix for 1) is to simple not define the height (since we've set the maxHeight to a reasonable value we can let the scrolling take care of itself... Fix for 2) was to add 'true' to the 'mapOffset' calls, causing them to correct for folded sections when computing the new offset. commit: 512f909aa1c81397d7aa1858958e2a903b17ce55 Silenio, I'll continue to test this tomorrow. For me it appears to be OK now (at least far better). Mike and I purposely limited the size of the tooltip as long JSDoc would make a tooltip covering too much of the page. I doubt your change will break that decision, but it is something to look at. Why did you change the offset calculation? Did you check what affect that change has on comment folding? Can you cc me on any editor tooltip changes? I've put a lot of effort in that area so we could pair up as well. http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=512f909aa1c81397d7aa1858958e2a903b17ce55 Just tested the fix, causes a problem if you have a large tooltip near the bottom of the screen. Rather than shifting the toolip above the anchor text the tooltip opens up over top of it. The solution you are probably looking for is to increase the default size (see defWidth tooltip.js:452). However as I mentioned we chose a default value to keep the size reasonable in Bug 468503. Not setting the height means the tooltip will often grow to the height of the editor. I have reverted the change. http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=cf58885d08331eb838da571b0f9cc53bc9264038 I have increased the default height of the tooltip. Please check if that works well for you. I just noticed a related issue. At default zoom tooltip.js:show() JSDoc fits nicely into the tooltip (no scrolling needed to see all content). However, if you mouse over the tooltip scrollbars are shown. Looks like the content margins can't quite fit inside the tooltip. Curtis, my bad for not copying you... The offset change was explicitly to account for cases where some jsdoc above where you're hovering is closed. Without the change (the 'true' was to tell 'mapOffset' to take folding into account) if you close (fold) some comments and hover over something below them then the hover is in the wrong place. I'm not sure how we can grow tooltips to cover the page since 'maxHeight' is 1/2 the viewport... "Rather than shifting the toolip above the anchor text the tooltip opens up over top of it."...this is a defect in the placement determination isn't it ? I actually do like the code structure (I can follow it) but I think that the decision of whether to place the tip above or below the anchor should be made *after* we've determined the height (once the width is set then you can reget the 'bounds' to see what the height is). Here's the commit for the 'mapOffset' bug: http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=68239cace8eaf070b06cbbcbe21249215b63f8a7 Here's another cut at fixing the clipping issue: http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=ea8ae02ee8ea4560b9ec45100248e9e9db3cbe5b Curtis, this should take care of most of your scenarios, please retry it...also, by moving the (re)definition of 'tipRect.height' to before we test if it'll 'fit' it also solves the issue where large JSDoc near the bottom will be correctly placed above the anchor (and limited to at most 1/2 the viewport). OK, I've pushed a new try...the previous one worked unless 'allowFullWidth' was set in which case 'defHeight' wasn't defined leading to the height being NaN. This had the odd effect of not being able to move the mouse into nav hovers (likely because the rect was pooched). I also moved the code above the 'Hack' to ensure that we don't break it... We have to revisit this code after 9 goes out to see if we can remove some of the constants...for right now if it works we should go with it... Link for this commit: http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=8a854dc3ba2405cd9b01a4ece946d8367f49caca I tested the latest on a variety of hovers and found no issues. The change to mapOffset does not fix Bug 463018, so I'm not sure what it is fixing. Nothing else planned here, marking as FIXED. |
Created attachment 254448 [details] Screen cap of the issue We aren't allowing enough height to show the JSDoc in many instances even though there's plenty of room to do so...