Opened 12 years ago

Closed 9 years ago

Last modified 5 years ago

#4282 closed defect (fixed)

Tooltip: places the tooltips wrong when window is very small.

Reported by: guest Owned by: bill
Priority: high Milestone: 1.6
Component: Dijit Version: 0.9
Keywords: Tooltip Cc: Katie Vance
Blocked By: Blocking:

Description (last modified by bill)

Test the following example. Make the window very small so that the lines in the tooltip and from the text right to the checkboxes will be broken. The tooltip will be positioned far away from the checkbox.

Attachments (5)

test.html (1.8 KB) - added by bill 11 years ago.
put in root directory, next to dojo/ and dijit/
test.2.html (1.8 KB) - added by bill 9 years ago.
further simplification of test
4282.patch (33.4 KB) - added by haysmark 9 years ago.
Fixes #4282. Refactor Tooltip_placement to use 1/5th the LOC.
place13.patch (47.2 KB) - added by Katie Vance 9 years ago.
This new patch runs on 400X400 min browser size.
place14.patch (48.0 KB) - added by Katie Vance 9 years ago.
Cumulative patch that replaces place13

Download all attachments as: .zip

Change History (30)

comment:1 Changed 12 years ago by bill

Milestone: 1.1

comment:2 Changed 12 years ago by bill

Owner: set to bill

comment:3 Changed 11 years ago by bill

Description: modified (diff)
Milestone: 1.11.2

comment:4 Changed 11 years ago by bill

Description: modified (diff)
Milestone: 1.21.4
Summary: Tooltip places the tooltips wrong when window is very small.Tooltip: places the tooltips wrong when window is very small.

I had to update your test file some (attached the new version) but I am able to reproduce.

Changed 11 years ago by bill

Attachment: test.html added

put in root directory, next to dojo/ and dijit/

comment:5 Changed 11 years ago by bill

Milestone: 1.4future

Changed 9 years ago by bill

Attachment: test.2.html added

further simplification of test

comment:6 Changed 9 years ago by bill

Milestone: future1.6

comment:7 Changed 9 years ago by bill

Resolution: fixed
Status: newclosed

(In [22969]) Tooltip positioning fixes:

  • Before positioning tooltip, limit tooltip's width to available width between the around node and the edge of the viewport. Reducing a tooltip's width (for tooltips with flowing text) will increase the tooltip's height, which affects vertical placement.
  • Position tooltip connector arrow to line up with the around node, even when tooltip (due to a large tooltip size/small viewport size) extends both above and below the around node.
  • Fix problem with tooltip placement when clicking a TextBox causes it to scroll into view simultaneously with displaying a tooltip.

Note: To determine the "natural" width of a toolip, had to remove left:50% setting for offscreen tooltips, because offscreen tooltips still get limited in width to the edge of the viewport. That CSS was apparently to work around a bug in an old browser (but no longer happens in the browsers we support).

Patch from Katie Vance (IBM, CCLA), thanks!

Fixes #4997, #4282 !strict.

Known issues: regression test failing by a few pixels on Mac FF (but working on Windows FF).

comment:8 Changed 9 years ago by haysmark

Resolution: fixed
Status: closedreopened

Bill, the new Tooltip_placement test has a lot of redundant test case declarations. I attached a patch to make the test file more maintainable.

comment:9 Changed 9 years ago by bill

Cc: Katie Vance added

Thanks, that looks like a much cleaner design (for the test), but even before [22995], I'm getting 4 failures (tested on IE6 and FF3.6/win). Not sure if those indicate a problem w/the test or with the code?

After [22995], I'm getting many test failures.

comment:10 Changed 9 years ago by haysmark

Yes before [22995] I was getting a few failures because the tooltip wasn't appearing on screen for very long textboxes. Now the _abs and _ab tests systematically fail for me.

Changed 9 years ago by haysmark

Attachment: 4282.patch added

Fixes #4282. Refactor Tooltip_placement to use 1/5th the LOC.

comment:11 Changed 9 years ago by haysmark

I fixed those 4 failures and reattached the patch. It turned out that one of the textboxes wasn't supposed to be tested, so 1 textbox*4 groups=4 failures. Also, sometimes I noticed the robot took an extremely long time to click the above,below button; this would cause the _abs and _ab tests to fail. Restarting the browser fixed it for me.

comment:12 Changed 9 years ago by bill

Resolution: fixed
Status: reopenedclosed

(In [23000]) Consolidating lots of repeated test code, patch from Mark Hays (IBM, CCLA), thanks! Fixes #4282.

comment:13 Changed 9 years ago by Douglas Hays

(In [23001]) Refs #4282. Add test actions based on events to greatly speed up the test. I'm getting no failures on IE6, FF3.6, Safari 5.0.

comment:14 Changed 9 years ago by Douglas Hays

(In [23015]) Refs #4282. Change test_Tooltip_placement.html to work better with small browser window sizes and with IE6, to use claro by default, and to validate against HTML5.

comment:15 Changed 9 years ago by Katie Vance

Resolution: fixed
Status: closedreopened

Doug, I like the idea of using <br>'s. I think it may have uncovered a real bug. However, we still need the tooltips to have more content because they must span the entire width available in order to test the main test case. Making them as small as you did caused the tests to fail on a normal size window. I'm attaching a patch to make them just a bit larger. The patch also addresses a bug I found after I resized the tooltips.

comment:16 Changed 9 years ago by Katie Vance

I'm also seeing the tests fail on IE8. I think my browser is running the last sets of tests so fast that they are validating the placement of the tooltip before the tooltip is placed. I will look into that tomorrow.

comment:17 Changed 9 years ago by Katie Vance

place13.patch is a cumulative patch that includes patches 11 and 12. I deleted both patches for 11 & 12 for that reason.

comment:18 Changed 9 years ago by Katie Vance

The new placement tests require a min browser size of 500px X 500px. Any smaller and some of the very small tooltips will be forced to overlap textboxes and fail tests because of the new nowrap behavior. Only the nowrap tests are allowed to overlap textboxes.

Changed 9 years ago by Katie Vance

Attachment: place13.patch added

This new patch runs on 400X400 min browser size.

Changed 9 years ago by Katie Vance

Attachment: place14.patch added

Cumulative patch that replaces place13

comment:19 Changed 9 years ago by bill

Resolution: fixed
Status: reopenedclosed

(In [23065]) Fix from Katie so that test runs again in a large browser window (not just a very small one), plus some fixes to the code itself. Fixes #4282 !strict.

comment:20 Changed 9 years ago by bill

(In [23288]) Patch from Katie to avoid spurious failure when browser maximized (FF on my machine has dojo.window.getBox().w --> 1583px), refs #4282.

comment:21 Changed 8 years ago by bill

(In [23520]) Don't let bottom of tooltip connector extend past bottom of tooltip content, as it makes the tooltip look broken. Problem occurred with <14px tall aroundNodes such as points on charts.

Would be better to center the tooltip content relative to the center of the aroundNode, rather than the current behavior where their bottoms are aligned. With this checkin, for small aroundNodes (such as points on charts) the connector doesn't quite vertically align with the aroundNode.

Refs #4282, #4997, [23512], !strict.

comment:22 Changed 8 years ago by bill

(In [23531]) Don't let top of tooltip connector extend past top of tooltip content, as it makes the tooltip look broken. Problem occurred with large aroundNodes such as the bubble charts in dojox/charting/tests/test_event2d.html.

Refs #4282, #4997, [23512], !strict.

comment:23 Changed 8 years ago by bill

In [26937]:

Rewrite Tooltip-placement.html, removing unnecessary dependency on dijit.form.ValidationTextBox? etc. Calling dijit.Tooltip.show() should be sufficient for testing tooltip placement, and will hopefully have less intermittent failures than the old test. Refs #4282 !strict.

comment:24 Changed 8 years ago by bill

In [26939]:

Remove code from [23065] to "Adjust width for tooltips that have a really long word or a nowrap setting". It doesn't work, as shown by "test14" failure in Tooltip-placement.html. It will make any tooltip with a sufficient amount of text expand to fill the width of the viewport, even if the tooltip content has neither large words nor a nowrap setting. Refs #4282 !strict.

comment:25 Changed 5 years ago by Bill Keese <bill@…>

In ff4671ee94323a629ecc09d4b6fffdecb2a72fee/dijit:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 
Note: See TracTickets for help on using tickets.