Opened 8 years ago

Closed 8 years ago

#14851 closed defect (fixed)

TooltipDialog: should support same orientations as Tooltip

Reported by: Karl Tiedt Owned by: bill
Priority: undecided Milestone: 1.8
Component: Dijit Version: 1.7.2
Keywords: Cc:
Blocked By: Blocking:

Description (last modified by bill)

Recently Tooltip's were updated to include a TM/BM (above-centered and below-centered) but TooltipDialogs do not inherit this same code so they do not support it

(Maybe the real fix should be to normalize and share that code between them?)

Change History (5)

comment:1 Changed 8 years ago by bill

Description: modified (diff)
Owner: set to Karl Tiedt
Status: newpending

Hmm, well _HasDropDown (and thus DropDownButton) has the dropDownPosition parameter that can be set to ["below-centered", "above-centered"]. The default is still ["below", "above"] though. Not sure if it makes sense to change the default? The current behavior matches how we drop-down menus.

Or you mean that dropDownPosition didn't work for you?

comment:2 Changed 8 years ago by Karl Tiedt

Status: pendingnew

I'll check that out, the code I was given was using the same open/orient syntax as Tooltip, since I will be at work tomorrow after all, I can test it first thing in the morning.

comment:3 Changed 8 years ago by Karl Tiedt

Actually dropDownPosition wouldnt work in our case (after looking at it more closely). This case is actually a TooltipDialog? that mimics a tooltip to allow for inlineEditing to occur. It appears the problem stems from TooltipDialog?'s orient() method not handling the new options, our dev was able to solve the problem by extending TooltipDialog? in this way:

    // Overriding the orient function so we could support 'M' orientation along with the existing 'L'&'R'.
    orient: function(/*DomNode*/ node, /*String*/ aroundCorner, /*String*/ corner){
        
        if (corner.charAt(1) == 'M'){
            
            var newC = "dijitTooltipABCenter"
                    + " dijitTooltip"
                    + (corner.charAt(0) == 'T' ? "Below" : "Above");
            
            dojo.replaceClass(this.domNode, newC, this._currentOrientClass || "");
            this._currentOrientClass = newC;
            
        }else{ // 'L' / 'R'
            this.inherited(arguments);
        }
    },

comment:4 Changed 8 years ago by bill

Milestone: tbd1.8
Owner: changed from Karl Tiedt to bill
Status: newassigned
Summary: TooltipDialog should support same orientations as TooltipTooltipDialog: should support same orientations as Tooltip

Ah, you are right.

This code is a mess, in that Tooltip and TooltipDialog have obvious similarities, but they are separate code paths (including separate orient() methods) because they have slightly different implementations, specifically that TooltipDialog uses dijit/popup, and Tooltip just supplies it's own iframe and goes directly to dijit/place. Someday I'll need to refactor the two classes to share code.

But in the meantime I'll add support to orient(), as you mentioned, plus the code to fix the connector to be in the middle of the TooltipDialog.

comment:5 Changed 8 years ago by bill

Resolution: fixed
Status: assignedclosed

In [27973]:

Support dropDownPosition: ["below-centered", "above-centered"] for parent of TooltipDialog (typically DropDownButton). In the future though Tooltip and TooltipDialog should be refactored to share more code. Fixes #14851 !strict.

Note: See TracTickets for help on using tickets.