Closed Bug 953538 Opened 7 years ago Closed 7 years ago

User should not be able to double click on connect / disconnect

Categories

(Instantbird :: Account manager, defect)

0.1.2
defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: romain, Assigned: romain)

Details

Attachments

(1 file, 1 obsolete file)

*** Original post on bio 87 at 2008-08-10 02:31:00 UTC ***

User should not be able to double click on connect / disconnect button on account manager.

When the user clicks, the button triggers and then being "disabled" for about 500ms, and then change to connect / disconnect.
Attached patch patch (obsolete) — Splinter Review
*** Original post on bio 87 as attmnt 43 at 2008-08-10 05:12:00 UTC ***

Added a setTimeout for connect/disconnect button.
Comment on attachment 8351787 [details] [diff] [review]
patch

*** Original change on bio 87 attmnt 43 at 2008-08-10 05:44:00 UTC ***

>Index: instantbird/base/content/instantbird/account.xml
>===================================================================
>--- instantbird/base/content/instantbird/account.xml	(revision 259)
>+++ instantbird/base/content/instantbird/account.xml	(working copy)
>@@ -85,11 +85,13 @@
>           <xul:button class="disconnectButton"
> 		      label="&account.disconnect.label;"
> 		      accesskey="&account.disconnect.accesskey;"
>-                      oncommand="gAccountManager.disconnect()"/>
>+                      oncommand="gAccountManager.disconnect()"
>+                      anonid="disconnect"/>
>           <xul:button class="connectButton"
> 		      label="&account.connect.label;"
> 		      accesskey="&account.connect.accesskey;"
>-                      oncommand="gAccountManager.connect()"/>
>+                      oncommand="gAccountManager.connect()"
>+                      anonid="connect"/>
>           <xul:button label="&account.delete.label;"
> 		      accesskey="&account.delete.accesskey;"
>                       oncommand="gAccountManager.delete()"/>

Please do something to fix the weired indentation here. Tabulations should be replaced byt spaces I guess... 

>+          var disconnect = document.getAnonymousElementByAttribute(this, "anonid", "disconnect");
>+          disconnect.setAttribute("disabled", "true");
>+
>+          window.setTimeout(function(disconnectBase) {disconnectBase.removeAttribute("disabled");},
>+                            this._disabledDelay, disconnect);

- You don't need the "window." here.

- For function arguments, usually the names are prefixed with "a".

- "disconnectBase" is not very explicit here. What does "base" mean here? I think I would use aDisconnectButton.

- The line looks a little too long and embedding code inside a function call line is not very readable.
I suggest (that's what I used everywhere else):
          setTimeout(function(aParameterName) {
                       // do something
                     }, delay, parameter);

>+          var connect = document.getAnonymousElementByAttribute(this, "anonid", "connect");
>+          connect.setAttribute("disabled", "true");
>+
>+          window.setTimeout(function(connectBase) {connectBase.removeAttribute("disabled");},
>+                            this._disabledDelay, connect);

The same comments apply here too.

It looks good. r=me with the coding style details fixed. I'd like to try it before landing though, to make sure I like the 500ms value for the delay.
Attachment #8351787 - Flags: review+
Attached patch patch V2Splinter Review
*** Original post on bio 87 as attmnt 44 at 2008-08-10 16:51:00 UTC ***

Added some indentation correction too.
Attachment #8351788 - Flags: review?
Assignee: florian → romain
Comment on attachment 8351788 [details] [diff] [review]
patch V2

*** Original change on bio 87 attmnt 44 at 2008-08-10 16:56:27 UTC ***

Great!
Attachment #8351788 - Flags: review? → review+
Comment on attachment 8351787 [details] [diff] [review]
patch

*** Original change on bio 87 attmnt 43 at 2008-08-10 16:56:58 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351787 - Attachment is obsolete: true
Status: NEW → ASSIGNED
*** Original post on bio 87 at 2008-08-13 22:14:50 UTC ***

Sending        instantbird/base/content/instantbird/account.xml
Transmitting file data .
Committed revision 275.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Summary: User should not ne able to double click on connect / disconnect → User should not be able to double click on connect / disconnect
Target Milestone: 0.2 → 0.1.3
You need to log in before you can comment on or make changes to this bug.