Opened 10 years ago
Closed 10 years ago
#9162 closed enhancement (fixed)
[patch] [cla] dijit.Tooltip - add addTarget/removeTarget and onShow/onHide
| Reported by: | alle | Owned by: | bill |
|---|---|---|---|
| Priority: | high | Milestone: | 1.4 |
| Component: | Dijit | Version: | 1.3.0 |
| Keywords: | Cc: | ||
| Blocked By: | Blocking: |
Description
Attached patch add addTarget/removeTarget methods for easier dynamic node connection, do some cleanup to prevent dynamic connection result in troubles and add onShow/onHide events.
Attachments (3)
Change History (6)
Changed 10 years ago by
| Attachment: | tooltip.diff added |
|---|
comment:1 follow-up: 2 Changed 10 years ago by
| Milestone: | tbd → 1.4 |
|---|---|
| Owner: | set to bill |
| Status: | new → assigned |
Hi Robert,
Thanks for the patch. I'm modifying it some to remove duplicate code and duplicate variables.
The thing is that it's hard to test the changes. Ideally we would have an automated test for this, although at a minimum there should be testing code in test_Tooltip.html. Can you add that? And also, test the changes I made to your patch in your own environment to make sure I didn't break anything.
Changed 10 years ago by
| Attachment: | tooltip_modified.diff added |
|---|
modified to remove some duplicate code and duplicate variables storing data about connections
Changed 10 years ago by
| Attachment: | tooltip_test.diff added |
|---|
Tests for dynamic target add/remove and onShow/onHide events
comment:2 Changed 10 years ago by
Modified patch works ok in my environment.
I have no experience in dojo automated tests so attached a patch for test_Tooltip.html to test new changes. Let me know if it's ok.
comment:3 Changed 10 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
(In [17437]) Add addTarget()/removeTarget() methods to connect/disconnect tooltip from additional DOMNodes, plus made Tooltip.attr('connectIds', ...) work correctly (to reset the list of nodes connected to).
Also added onShow/onHide events.
Fixes #9162 !strict.
Based on patch from alle (CLA on file).
ops ... I've attached old version, this is the correct one