Closed Bug 456379 Opened 11 years ago Closed 11 years ago

Thunderbird3: Remove lightning calendar/task toolbars

Categories

(Calendar :: Calendar Views, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: chris.j.bugzilla, Assigned: Fallen)

References

Details

Attachments

(6 files, 4 obsolete files)

Due to the toolbar consolidation the following buttons need to find a new home:

* Move Category,
* Mark Completed
* Priority into Task View

An idea could be to move them below the "New Task" Edit Field of the Task View, but this needs to be sketched out more in detail. As always any suggestions are welcome.
Flags: tb-integration?
Flags: tb-integration? → tb-integration+
Flags: blocking-calendar1.0+
Priority: -- → P1
This needs some UI to move forward
Assignee: nobody → clarkbw
Here is some ui which aligns with the current Shredder look. Please find the proposal attached. I've also fine tuned the borders a little bit, but this is another issue :-)
Comment on attachment 348176 [details]
Task Buttons Moved from toolbar into view

This looks like a good step forward.

I think we can improve the look of both this and the message reader, but we've pushed that off the plate until later (post b1).
Attachment #348176 - Flags: ui-review+
I disagree with the placement of the new task button. Beginner users might think they need to enter a text into the textbox and then click on the button to create the new task.

Of course we could use the text from the textbox, if it was the last element focused, but that seems a bit hacky codewise.
Attached patch Fix - v1 (obsolete) β€” β€” Splinter Review
This patch takes care of all but the "New Task" button. Note that for some reason hg qdiff is also diffing deleted files for me, but they are marked as deleted with this patch anyway.

This patch also takes care of removing toolbars as a whole for both calendar and task mode (bug 454935) and improves the situation for modifying/deleting the selected item in Sunbird. For the latter issue there are still some things open (might be possible to take care via theme changes), but its better than it was before.

The mail theme changes are to allow also menu-buttons styled the way the message header buttons are. I haven't been able to test this on mac, but aside from that this shouldn't change any looks in the mail view, just add the styles for the button in the task view.

What I haven't taken care of is making attendee/organizer fields use the bindings to show multiple email addresses and allow starring the attendees. I'll leave this open for a different bug that consolidates those bindings to be usable outside of mail-land.

Also, there are some small mismatches in casing and use of the colon for the Start/End/Due date rows in the task view header. This is due to the fact that those rows use bindings that are very strict about the label. Berend, can I file a followup bug assigned to you to fix this, since you created the binding?

I'll either file a followup bug for adding new task (and what about "edit task"? We need a location for this possibly), or integrate that into this patch if a sufficient solution is found before final review.
Assignee: clarkbw → philipp
Status: NEW → ASSIGNED
Attachment #348550 - Flags: review?(Berend.Cornelius)
This patch requires at least v4 of the patch for bug 454933.
Blocks: 454935
Depends on: 454933
Attached image Screenshot - v1 β€”
Attachment #348551 - Flags: ui-review?(christian.jansen)
Comment on attachment 348550 [details] [diff] [review]
Fix - v1

ui-r? and r? for the mail theme changes
Attachment #348550 - Flags: ui-review?(clarkbw)
Attachment #348550 - Flags: review?(bugzilla)
Comment on attachment 348551 [details]
Screenshot - v1

The buttons are looking good for me. It would be nice if we could integrate the "Add Task" Button in front of the QuickTask Field within this patch.

I personally do not like that the title, from, priority, status category strings do not start with a capital letters. In addition they should end with an ":". It should be possible to star the sender (From), just for being constant to the mail header.
Attachment #348551 - Flags: ui-review?(christian.jansen) → ui-review+
(In reply to comment #10)
> (From update of attachment 348551 [details])
> The buttons are looking good for me. It would be nice if we could integrate the
> "Add Task" Button in front of the QuickTask Field within this patch.
I'll do that in the final patch, please pretend it has been done.

> 
> I personally do not like that the title, from, priority, status category
> strings do not start with a capital letters. In addition they should end with
> an ":". 

I took this from the mail headers to be consistant, I guess you and Bryan should agree on something in that area

> It should be possible to star the sender (From), just for being
> constant to the mail header.
As noted, different bug.
Attached patch Fix - v2 (obsolete) β€” β€” Splinter Review
de-bitrotted, doesn't depend on anything since mentioned bug is fixed. Includes add-task button.
Attachment #348550 - Attachment is obsolete: true
Attachment #348593 - Flags: ui-review?(clarkbw)
Attachment #348593 - Flags: review?(Berend.Cornelius)
Attachment #348550 - Flags: ui-review?(clarkbw)
Attachment #348550 - Flags: review?(bugzilla)
Attachment #348550 - Flags: review?(Berend.Cornelius)
Attachment #348593 - Flags: review?(bugzilla)
Also fixes bug 462187 and bug 461132, at least partially and probably also bug 412809
Blocks: 461132, 462187, 412809
Attachment #348593 - Flags: review?(Berend.Cornelius) → review+
Comment on attachment 348593 [details] [diff] [review]
Fix - v2

The patch works great and looks very good, too.
Some notes:

>.cal-toolbarbutton-menubutton-button[open="true"] {
>   padding-top: 6px;
>   padding-bottom: 5px;
>   -moz-padding-start: 5px;
>   -moz-padding-end: 4px;
>}

>.cal-toolbarbutton-1[checked="true"] {
>   padding-top: 6px !important;
>   padding-bottom: 5px !important;
>   -moz-padding-start: 5px !important;
>   -moz-padding-end: 4px !important;
>}

Can't these rules be consolidated? I haven't checked if "!important" is really necessary here. But this is not your actual issue.

I find that the labels should be capitalized as ever, but maybe this is just because I am used to it.

The delete-button is one pixel higher than the other buttons. Maybe you can add a 1px margin at the button-text.

"Mark Completed" Button is jumping when menu pops up (at least a very very little).

<!ENTITY calendar.task-details.startdate.label       "title"> 

is not used and wrong...

The priority and category buttons do not have a "disabled" rule
r=berend
(In reply to comment #14)
> Can't these rules be consolidated? I haven't checked if "!important" is really
> necessary here. But this is not your actual issue.

I just moved these rules, I'll see if I can consolidate them.


> 
> I find that the labels should be capitalized as ever, but maybe this is just
> because I am used to it.
...
> The priority and category buttons do not have a "disabled" rule

These two issues needs a decision from Bryan and Christian. Temporarily re-assigning to get traction.

I'll fix the remaining noted issues when a decision is out.
Assignee: philipp → clarkbw
Comment on attachment 348593 [details] [diff] [review]
Fix - v2

I'm going to defer this reviews, I think the mail/ changes look fine, but I'd like Phil to check just in case. Also something is wrong in the task view on Mac, I'll attach a screenshot in a moment.
Attachment #348593 - Flags: review?(bugzilla) → review?(philringnalda)
Attached image Mac Task Header β€”
This layout doesn't seem quite right.
I unfortunately won't have a mac until monday, so I can't really fix this issue :-( I would appreciate if someone with a mac could take a look!

I was thinking, maybe "disabled" should hide the buttons in this case. a readonly task doesn't really need extra icons, right?
> I was thinking, maybe "disabled" should hide the buttons in this case. a
> readonly task doesn't really need extra icons, right?

Yes, that's consistent with what we're doing for buttons in the message reader view.

> I find that the labels should be capitalized as ever, but maybe this is just
> because I am used to it.

We moved away from capitalized header labels in the message view because they detract from the actual content, notably the more important part.  Much of this preference seems to depend on the person seeing the label as starting the sentence or a piece of meta-data.
(In reply to comment #19)
> > I find that the labels should be capitalized as ever, but maybe this is just
> > because I am used to it.
> 
> We moved away from capitalized header labels in the message view because they
> detract from the actual content, notably the more important part.  Much of this
> preference seems to depend on the person seeing the label as starting the
> sentence or a piece of meta-data.

I agree that they detract to some degree, and I'm fine with the color change (as long as the change color again in high contrast modes -> accessibility)

But my point of view the meta-data approach is pretty technical. IMO labeling should be consistent for all UI elements. Why are these labels different to labels used for UI elements on dialogs? Same things should behave/look same -- even if it's terminology related.
No longer blocks: 461132
Duplicate of this bug: 461132
Attached patch Fix - v3 (obsolete) β€” β€” Splinter Review
Attachment #348593 - Attachment is obsolete: true
Attachment #348593 - Flags: ui-review?(clarkbw)
Attachment #348593 - Flags: review?(philringnalda)
Attached patch Fix - v4 (obsolete) β€” β€” Splinter Review
This patch takes care of the mac problems and also aligns the qute/pinstripe files where needed. The buttons may be a bit smaller than before on mac, I decided to align designs on mac/windows in that area. It also hides the buttons when they are are disabled.

This patch doesn't take care of aligning the capitalization, bryan please respond to Christian's comment.

Requesting review for the mail theme changes.
Attachment #349953 - Attachment is obsolete: true
Attachment #349955 - Flags: ui-review?(clarkbw)
Attachment #349955 - Flags: review?(philringnalda)
> I agree that they detract to some degree, and I'm fine with the color change
> (as long as the change color again in high contrast modes -> accessibility)
> 
> But my point of view the meta-data approach is pretty technical. IMO labeling
> should be consistent for all UI elements. Why are these labels different to
> labels used for UI elements on dialogs? Same things should behave/look same --
> even if it's terminology related.

I'm not sure I agree that the same labels are being used in dialogs or perhaps I don't understand what you're referring to.  I think this is strict structure vs. structure inferred from format.

In our message reader view the lowercase header labels are shown in a secondary view.  The initial collapsed default view for the message reader doesn't include labels at all as it's intended to read like a real message header. (i.e. when a person writes you a letter they don't label their headers, they use formatting to denote what is what)

In the secondary, detailed view we show the header labels because the intent is to display more detail than the formatted view.  So I think in our message reader these labels are meta-data to the secondary view, and therefore we tuned down the attraction they receive.

I don't think tasks yet have a formatted view and detailed view, however I think we need to keep the same look and format between both.

As a late l10n trick you can still add CSS to use the text-transform: lowercase; on your header labels.
Since the lowercase issue is something that can be fixed in a followup bug, please do consider reviewing the mail changes soon. The mail changes do not including any case changes. I wouldn't want this patch to bitrot since it is quite large and also some other bugs depend on it being applied!!
Duplicate of this bug: 454935
Hardware: PC → All
Summary: Thunderbird3: Move Category, Mark Completed, Priority into Task View → Thunderbird3: Remove lightning calendar/task toolbars
Comment on attachment 349955 [details] [diff] [review]
Fix - v4

assuming we can get the labels fixed in the next patch this looks like a good first part.
Attachment #349955 - Flags: ui-review?(clarkbw) → ui-review+
Assignee: clarkbw → philipp
Attachment #349955 - Flags: review?(philringnalda) → review+
Comment on attachment 349955 [details] [diff] [review]
Fix - v4

Sorry, I'm used to being the slow one holding up the process, but I'm not used to it being on bugs where I'm not watching the QA and getting the throat-clearing bugmail. The mail bits look fine to me (other than the little bit of rot you've picked up, but it doesn't look like it'll be too bad to fix).
Comment on attachment 349955 [details] [diff] [review]
Fix - v4

You'd think I'd have enough sense to wait for my build to finish, rather than reviewing how I think it's going to work, wouldn't you?

At least on OS X, this jams the messagereader buttons up against the top of the frame without any space above them, and makes the text buttons smaller than the trash icon button, neither of which really sound like what we want to do.
Attachment #349955 - Flags: review+
OSX again, sigh. I'll see what I can do next week when I have access to the mac again.
Attached patch Fix - v5 β€” β€” Splinter Review
This patch fixes the mac issue from the previous patch.

requesting review for the mail changes. I'd enjoy a speedy review!
Attachment #349955 - Attachment is obsolete: true
Attachment #353036 - Flags: ui-review+
Attachment #353036 - Flags: review?(philringnalda)
Comment on attachment 353036 [details] [diff] [review]
Fix - v5

Thanks, that'll work. Lucky for you the context of your patch shows we already have a ton of RTL-breaking "-left" and "foo: 1px 2px 3px 4px" so I can't justify making you fix it all :)
Attachment #353036 - Flags: review?(philringnalda) → review+
Thanks for reviewing! I didn't know "foo: a b c d" kills RTL, but now that you mention it, that totally makes sense. I'll keep this in mind in future patches.


Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/7d617031aa60>

-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0
here: http://hg.mozilla.org/comm-central/rev/7d617031aa60#l13.60
+<!ENTITY calendar.task-details.startdate.label       "title">
+<!ENTITY calendar.task-details.title.label           "title">

Is this a mistake?
Yes, its a mistake. Thanks for catching this so quickly! The first entity is not used, I'm removing it now:

<http://hg.mozilla.org/comm-central/rev/dfa33743cfba>
On my WinXP I have a problem similar to that one reported in comment #17: 'mark completed' button looks strange, it seems there are two overlapped buttons  (see attachment).
Are you using a compiled thunderbird? if so you need to rebuild mail/themes. Otherwise - argh!

Andreas, could you also give this a test on windows, specifically?
Attached image wrong sized buttons on WinXP β€”
Looks different on my WinXP. The "Mark Completed" button is smaller than all the other ones
After an automatic update of Shredder and installed the latest Lightning, on a fresh profile, I have the same situation as in previous comment.
Decathlon, I suggest that you file a new bug for the issue and mark it blocking this bug.
Depends on: 470824
Filed bug 470824 for the Windows issue.
Blocks: 472408
Target Milestone: 1.0 → 1.0b1
You need to log in before you can comment on or make changes to this bug.