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)

tooltip.diff (4.1 KB) - added by alle 10 years ago.
ops ... I've attached old version, this is the correct one
tooltip_modified.diff (3.3 KB) - added by bill 10 years ago.
modified to remove some duplicate code and duplicate variables storing data about connections
tooltip_test.diff (2.6 KB) - added by alle 10 years ago.
Tests for dynamic target add/remove and onShow/onHide events

Download all attachments as: .zip

Change History (6)

Changed 10 years ago by alle

Attachment: tooltip.diff added

ops ... I've attached old version, this is the correct one

comment:1 Changed 10 years ago by bill

Milestone: tbd1.4
Owner: set to bill
Status: newassigned

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 bill

Attachment: tooltip_modified.diff added

modified to remove some duplicate code and duplicate variables storing data about connections

Changed 10 years ago by alle

Attachment: tooltip_test.diff added

Tests for dynamic target add/remove and onShow/onHide events

comment:2 in reply to:  1 Changed 10 years ago by alle

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 bill

Resolution: fixed
Status: assignedclosed

(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).

Note: See TracTickets for help on using tickets.