195722 - Want simple event when splitter finishes drag or grippy clicked

Status: RESOLVED FIXED

(Core :: XUL, enhancement)

Description

[[e-mail]]

When a splitter finishes drag you can sort of tell by trapping the mouseup
event. When a grippy is clicked you can sort of tell by trapping the click
event. Unfortunately the click event happens before the click action, so that
the actual state of the splitter is reversed.

By reversing the timing of the click action and event and adding a command event
to the splitter's mouseup event then the splitter state can be guaranteed to be
current for a command event handler on the splitter.

Comment 1

[[e-mail]]

Attachment: Proposed patch

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 116087 [details] [diff] [review]
Proposed patch

1) Please document in this code why the event ordering has to be the way you're
making it.

2) Where is it documented that the command event is what one should trigger
off?  Should this be a comment in some XBL?  Should this be in the XUL widget
behavior documentation we have (we _do_ have some, right?)

3) just test for |splitter| instead of |splitter != nsnull|.

4) A diff -w would be much easier to review.  ;)

Comment 3

[[e-mail]]

1) Please document in this code why the event ordering has to be the way you're
making it.

You mean in the grippy code, right? Added:

// update the splitter first, in case someone's listening on the command event

2) Where is it documented that the command event is what one should trigger
off?  Should this be a comment in some XBL?  Should this be in the XUL widget
behavior documentation we have (we _do_ have some, right?)

I chose command because grippys already generate command events. This means that
you can write a command event handler on the splitter to watch for all splitter
adjustment notifications. I'll see what XUL documentation we have (if any :-)

3) just test for |splitter| instead of |splitter != nsnull|.

As you can tell I just reversed the sense of the test, I assumed that if style
was to use (splitter == nsnull) then it was to use (splitter != nsnull).

4) A diff -w would be much easier to review.  ;)

But then I wouldn't be able to point out my tab to space conversion :-P

Comment 4

[Boris Zbarsky [:bzbarsky]]

> I chose command because grippys already generate command events.

"Document that in the code".  And yes, I mean the grippy code.

I know you were following "the style of the file" with the nsnull.  Don't
please, in this case.

As for -w, the usual procedure is to attach both a normal diff and a -w one.  ;)

Comment 5

[[e-mail]]

Attachment: diff -w

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 116197 [details]
diff -w

sr=bzbarsky with two more nits:

1) NS_NAMED_LITERAL_STRING(collapsed,  "collapsed");  instead of the |nsString
a| we have now (what kind of variable name is "a"??)

2)  Make "value" an nsAutoString.

Comment 7

[[e-mail]]

bz, rather than me trying to second-guess you, or you trying to read diffs that
don't have enough context because I used -w, the JS for that code is:

splitter.setAttribute("state", splitter.getAttribute("state") == "collapsed" ?
"open" : "collapsed");

So, if you can just let me know what that should be in C++ please...

Comment 8

[Boris Zbarsky [:bzbarsky]]

Ah, I had missed you toggling the value.  In that case, just make it an
nsAutoString (and call it "state" or something instead of "a", ok?  ;))

As for context:

cvs diff -u10 -w 

will do what you want.... (and in fact that should be done even when not making
-w diffs).

Comment 9

[[e-mail]]

Attachment: updated diff -w

Comment 10

[[e-mail]]

Attachment: Updated patch

For checkin purposes. Plus fixed typo in previous diff -w

Comment 11

[[e-mail]]

Comment on attachment 116207 [details] [diff] [review]
Updated patch

review hokey-cokey

Comment 12

[Jan Varga [:janv]]

Comment on attachment 116207 [details] [diff] [review]
Updated patch

r=varga

Comment 13

[[e-mail]]

Attachment: Updated for bitrot

GetContent was changed, and I've been running with this revised diff since.

Comment 14

[[e-mail]]

Fix checked in.