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

RESOLVED FIXED in Thunderbird 3.1b2

Status

defect
--
minor
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: andreasn, Assigned: ewong)

Tracking

Trunk
Thunderbird 3.1b2

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

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"
yeah, I'd choose "Create folder"
With a capital F please
Severity: normal → minor
Component: General → Mail Window Front End
QA Contact: general → front-end
Whiteboard: [good first bug]
Assignee: nobody → edmund
- Changed the default "OK" button to 'Create Folder' as per 
  the description and comments.
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.
Attachment #428375 - Attachment is obsolete: true
Woohoo! Sweet! Let me know if you need a ui-review.
(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)!
Attachment #428386 - Flags: review?(nisses.mail)
Attachment #428386 - Flags: review?(nisses.mail) → ui-review?(nisses.mail)
OS: Linux → All
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.
(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?
Posted patch Fixed no newline issue. (obsolete) — Splinter Review
- Fixed the no newline.

- Added the appropriate tags in the newFolderDialog.xul as suggested by 
  Standard8.
Attachment #428386 - Attachment is obsolete: true
Attachment #428870 - Flags: review?(nisses.mail)
Attachment #428386 - Flags: ui-review?(nisses.mail)
(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...
Attachment #428870 - Attachment is obsolete: true
Attachment #428893 - Flags: review?(nisses.mail)
Attachment #428870 - Flags: review?(nisses.mail)
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 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 :)
Attachment #428893 - Flags: review?(nisses.mail) → ui-review?(nisses.mail)
(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.
Status: NEW → ASSIGNED
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.
Attachment #428893 - Flags: ui-review?(nisses.mail) → ui-review+
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.
Attachment #428893 - Flags: superreview?(neil)
Attachment #428893 - Flags: review?(bugzilla)
Attachment #428893 - Flags: superreview?(neil) → superreview+
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.
Attachment #428893 - Flags: review?(bugzilla) → review+
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
Attachment #428893 - Attachment description: Updated previous patch to include accesskey tags in the dtd files. → Updated previous patch to include accesskey tags in the dtd files [Checkin: Comment 18]
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Hardware: x86 → All
Resolution: --- → FIXED
Whiteboard: [good first bug]
Target Milestone: --- → Thunderbird 3.1b2
You need to log in before you can comment on or make changes to this bug.