495939 - New Folder dialog don't use a verb for the confirm button

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Andreas Nilsson (:andreasn)]

Attachment: screenshot of the current dialog

According to the Interface guidelines for both Vista, OS X and GNOME, it's
preferred to use a verb instead of a standard button labeled OK. 

I would suggest "Create" or "Create folder"

Comment 1

[Bryan Clark (DevTools PM) [:clarkbw]]

yeah, I'd choose "Create folder"

Comment 2

[Magnus Melin [:mkmelin]]

With a capital F please

Comment 3

[Edmund Wong (:ewong)]

Attachment: Change the default OK button to "Create Folder"

- Changed the default "OK" button to 'Create Folder' as per 
  the description and comments.

Comment 4

[Edmund Wong (:ewong)]

Attachment: Replacing previous patch after review by philor

The original patch was incorrect.  It should be a modification of both
the suite/locales/en-US/chrome/mailnews/newFolderDialog.dtd and
mail/locales/en_US/chrome/messenger/newFolderDialog.dtd instead.

Comment 5

[Andreas Nilsson (:andreasn)]

Woohoo! Sweet! Let me know if you need a ui-review.

Comment 6

[Ludovic Hirlimann [:Usul]]

(In reply to comment #4)
> Created an attachment (id=428386) [details]
> Replacing previous patch after review by philor
> 
> The original patch was incorrect.  It should be a modification of both
> the suite/locales/en-US/chrome/mailnews/newFolderDialog.dtd and
> mail/locales/en_US/chrome/messenger/newFolderDialog.dtd instead.

To get the patch in Edmund you need to follow https://developer.mozilla.org/en/comm-central#Requirements and as your patch touches UI you'll need a ui review (hint andreas volunteered)!

Comment 7

[Andreas Nilsson (:andreasn)]

I think there might be pieces missing from attachment 428386 [details] [diff] [review] that was in the first patch as I can't get this to work on two of my systems. We can look into it tomorrow on IRC.

Comment 8

[Edmund Wong (:ewong)]

(In reply to comment #7)
> I think there might be pieces missing from attachment 428386 [details] [diff] [review] that was in the
> first patch as I can't get this to work on two of my systems. We can look into
> it tomorrow on IRC.

Strange.  Is it because of the no newline at the end of the 2nd 
part of the diff?

Comment 9

[Edmund Wong (:ewong)]

Attachment: Fixed no newline issue.

- Fixed the no newline.

- Added the appropriate tags in the newFolderDialog.xul as suggested by 
  Standard8.

Comment 10

[Mark Banner (:standard8)]

(In reply to comment #9)
> - Added the appropriate tags in the newFolderDialog.xul as suggested by 
>   Standard8.

I think you forgot to add accept.accesskey to the dtd files...

Comment 11

[Edmund Wong (:ewong)]

Attachment: Updated previous patch to include accesskey tags in the dtd files [Checkin: Comment 18]

Comment 12

[Andreas Nilsson (:andreasn)]

Sweet, got it to work on one of my machines now!
You probably want to set me as ui-r, rather than r.

Who is best to set as (code) reviewer?
Magnus?

Comment 13

[Andreas Nilsson (:andreasn)]

Comment on attachment 428893 [details] [diff] [review]
Updated previous patch to include accesskey tags in the dtd files
[Checkin: Comment 18]

Setting myself as ui-reviewer, as I assume that was the intent. (I make mistakes like that all the time as well :)

Comment 14

[Edmund Wong (:ewong)]

(In reply to comment #13)
> (From update of attachment 428893 [details] [diff] [review])
> Setting myself as ui-reviewer, as I assume that was the intent. (I make
> mistakes like that all the time as well :)

Yes.  My bad.  It was supposed to be ui-reviewer.  c-c isn't compiling
on my box due to some manifest issue.

Comment 15

[Andreas Nilsson (:andreasn)]

Comment on attachment 428893 [details] [diff] [review]
Updated previous patch to include accesskey tags in the dtd files
[Checkin: Comment 18]

Looks good to me, now we need a code review from someone as well.

Comment 16

[Andreas Nilsson (:andreasn)]

Comment on attachment 428893 [details] [diff] [review]
Updated previous patch to include accesskey tags in the dtd files
[Checkin: Comment 18]

Setting Mark as code reviewer and neil as super reviewer as per Mark's suggestion on irc.

Comment 17

[Mark Banner (:standard8)]

Comment on attachment 428893 [details] [diff] [review]
Updated previous patch to include accesskey tags in the dtd files
[Checkin: Comment 18]

Looks good, thanks for the patch.

Comment 18

[Serge Gautherie (:sgautherie)]

Comment on attachment 428893 [details] [diff] [review]
Updated previous patch to include accesskey tags in the dtd files
[Checkin: Comment 18]


http://hg.mozilla.org/comm-central/rev/a55720d38ec1