Add icons to more imip bar buttons

RESOLVED FIXED in 4.0.0.1

Status

Calendar
Lightning Only
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: MakeMyDay, Assigned: MakeMyDay)

Tracking

Trunk
4.0.0.1

Details

Attachments

(3 attachments)

(Assignee)

Description

3 years ago
Created attachment 8414504 [details]
ImipBarIconsWin7.png

To follow up to bug 990009, we propbably need some more icon for the imipbar buttons.

These buttons currently don't have an icon:

.imipDetailsButton
-> open the event or summary dialog

.imipMoreButton
-> dropdown button to offer more options to the user

.imipAddButton
-> add an event to the calendar

.imipUpdateButton
-> ipdate an event in the calendar

.imipDeleteButton
-> delete an event from the calendar

.imipReconfirmButton
-> reconfirm the own participation status on organizer's request

Addtionally, these buttons share the same icons as those without recurrences, maybe they should also get different ones:

.imipAcceptRecurrencesButton
.imipDeclineRecurrencesButton
.imipTentativeRecurrencesButton
-> the same functionality as standard accept, deline, tentative buttons, but
   for recurring events

The respective css definitions are located at:

/calendar/base/themes/windows/win-aero/lightning.css
/calendar/base/themes/windows/win-classic/lightning.css
/calendar/lightning/themes/linux/lightning.css
/calendar/lightning/themes/osx/lightning.css

Last but not least, the icon used for accepting tentatively look a little bit blurry on Win 7 compared to the both others - see attached screenshot.
(Assignee)

Comment 1

3 years ago
@philipp: should we mark this also wanted for upcoming release for TB 31? It's not a must have, but people will will notice the mixture of buttons with and without icons.
Flags: needinfo?(philipp)
We would need icons to do that. Although unfortunate, I don't think we should burden the release with more changes than needed. I once did a seemingly simple CSS change shortly before the release and had to do a chemspill because the CSS change broke something unrelated and major.
Flags: needinfo?(philipp)
(Assignee)

Comment 3

3 years ago
As discussed on the summit with Paenglab, we should hide the space reserved for the icons from the buttons without icons until we have the icons. See bug 1089362.
(Assignee)

Comment 4

2 years ago
I have had a look, whether we can recyle some existing icons for the remaining buttons without icons. For Mac and Windows-Aero this seems not to hard:

+.imipAddButton {
+  list-style-image: url(chrome://calendar/skin/toolbar.png);
+  -moz-image-region: rect(0px 32px 16px 16px);
+}
+
+.imipAddButton:active {
+  -moz-image-region: rect(16px 32px 32px 16px);
+}
+
+.imipUpdateButton {
+  list-style-image: url(chrome://calendar/skin/toolbar.png);
+  -moz-image-region: rect(0px 16px 16px 0px);
+}
+
+.imipUpdateButton:active {
+  -moz-image-region: rect(16px 16px 32px 0px);
+}
+
+.imipDeleteButton {
+  list-style-image: url(chrome://calendar/skin/toolbar.png);
+  -moz-image-region: rect(0px 96px 16px 80px);
+}
+
+.imipDeleteButton:active {
+  -moz-image-region: rect(16px 96px 32px 80px);
+}
+
+.imipReconfirmButton {
+  list-style-image: url(chrome://calendar/skin/tasks-actions.png);
+  -moz-image-region: rect(0px 48px 16px 32px);
+}
+
+.imipReconfirmButton:active {
+  /*currently no separate image available*/
+}
+
+.imipMoreButton {
+  /* we should crop the image space of this button */
+}

For Linux and Win Classic, I'm not sure whether the corresponding icons from toolbar.png are really appropriate.

Richard, what do you think?
Flags: needinfo?(richard.marti)
I think this should work. Actually I'm on moving from png files to svg files for the toolbars. This would then use one shared file for Linux, OS X and Windows Vista+. I leave XP as it is because I think users which are still on XP don't want any change.
For now, the Linux and XP icons should also work also when they are a little different. For Linux this should change with my patch. I don't know when my patch can be checked-in as I'm awaiting the review for TB first (bug 1150627).

I have started a try build with the changes for TB and Lightning if you want to check it: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=d378ce077386

Every feedback is welcome.
Flags: needinfo?(richard.marti)
(Assignee)

Comment 6

2 years ago
(In reply to Richard Marti (:Paenglab) from comment #5)
> I think this should work.

So, you're satisfied with the association of the different icons to the respective buttons? (Sorry, I didn't manage to prepare a screenshot yet)

> Actually I'm on moving from png files to svg files
> for the toolbars. This would then use one shared file for Linux, OS X and
> Windows Vista+. I leave XP as it is because I think users which are still on
> XP don't want any change.

Is the new style similar to the current Mac/Win Aero style or different?

> For now, the Linux and XP icons should also work also when they are a little
> different. For Linux this should change with my patch. I don't know when my
> patch can be checked-in as I'm awaiting the review for TB first (bug
> 1150627).

Ok, I keep an eye on it, but I would love to get the icons to the imip bar button still for the next beta, so let's see.

> I have started a try build with the changes for TB and Lightning if you want
> to check it:
> https://treeherder.mozilla.org/#/jobs?repo=try-comm-
> central&revision=d378ce077386
> 
> Every feedback is welcome.

Unfortunately, I don't have a Mac around, but if you retrigger an all-plattform try build, I'm happy to give feedback.
(In reply to MakeMyDay from comment #6)
> (In reply to Richard Marti (:Paenglab) from comment #5)
> > I think this should work.
> 
> So, you're satisfied with the association of the different icons to the
> respective buttons? (Sorry, I didn't manage to prepare a screenshot yet)
> 
> > Actually I'm on moving from png files to svg files
> > for the toolbars. This would then use one shared file for Linux, OS X and
> > Windows Vista+. I leave XP as it is because I think users which are still on
> > XP don't want any change.
> 
> Is the new style similar to the current Mac/Win Aero style or different?

Yes, like the current Mac/Win Aero style.

> > For now, the Linux and XP icons should also work also when they are a little
> > different. For Linux this should change with my patch. I don't know when my
> > patch can be checked-in as I'm awaiting the review for TB first (bug
> > 1150627).
> 
> Ok, I keep an eye on it, but I would love to get the icons to the imip bar
> button still for the next beta, so let's see.

No problem, I can then update my patch.

> Unfortunately, I don't have a Mac around, but if you retrigger an
> all-plattform try build, I'm happy to give feedback.

For Win and Linux: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=a113818e2f82
(Assignee)

Comment 8

2 years ago
Created attachment 8591328 [details]
NewImipBarIcons.png

Attaching the selected icons - I have a patch ready for this.

The only missing thing is to crop the space in the More.. button. Richard, do you have an idea what inherited style needs to be overwritten for that?

Apart from that: I checked your Lightning SVG try build for Windows and have some findings. Do you already have a bug for that where I can comment?
Assignee: nobody → makemyday
Status: NEW → ASSIGNED
Flags: needinfo?(richard.marti)
(Assignee)

Updated

2 years ago
Target Milestone: --- → 4.0
(In reply to MakeMyDay from comment #8)
> Created attachment 8591328 [details]
> NewImipBarIcons.png
> 
> Attaching the selected icons - I have a patch ready for this.

This looks good. With Classic you mean XP, right?

> The only missing thing is to crop the space in the More.. button. Richard,
> do you have an idea what inherited style needs to be overwritten for that?

Add in calendar/lightning/themes/common/lightning.css:

.imipMoreButton > .toolbarbutton-icon {
  display: none;
}

> Apart from that: I checked your Lightning SVG try build for Windows and have
> some findings. Do you already have a bug for that where I can comment?

Not yet, but I'm filing one today and CC you.
Flags: needinfo?(richard.marti)
(Assignee)

Comment 10

2 years ago
Created attachment 8591339 [details] [diff] [review]
AddIconsToToolbarButtons-V1.diff

Yes, classic = XP.
Attachment #8591339 - Flags: review?(richard.marti)
Attachment #8591339 - Flags: approval-calendar-beta?(philipp)
Attachment #8591339 - Flags: approval-calendar-aurora?(philipp)
Attachment #8591339 - Flags: approval-calendar-beta?(philipp)
Attachment #8591339 - Flags: approval-calendar-beta+
Attachment #8591339 - Flags: approval-calendar-aurora?(philipp)
Attachment #8591339 - Flags: approval-calendar-aurora+
Attachment #8591339 - Attachment is patch: true
Comment on attachment 8591339 [details] [diff] [review]
AddIconsToToolbarButtons-V1.diff

Review of attachment 8591339 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. I have only tested Linux, XP and Win7 but from code OS X looks good.
Attachment #8591339 - Flags: review?(richard.marti) → review+
(Assignee)

Updated

2 years ago
Keywords: checkin-needed
Blocks: 1153752
https://hg.mozilla.org/comm-central/rev/e3416e8b3075

Uplifts to aurora and beta being done after the patch passes on central.
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
This still not landed on aurora and beta, so this fix is not in 4.0 afaik.

Updated

2 years ago
Keywords: checkin-needed
Whiteboard: [checkin-needed comm-aurora, comm-beta]
Target Milestone: 4.0 → 4.2
Backported to releases/comm-aurora changeset 0111428ad283
Keywords: checkin-needed
Target Milestone: 4.2 → 4.1
Backported to releases/comm-beta changeset 37812d78b380
Target Milestone: 4.1 → 4.0
Whiteboard: [checkin-needed comm-aurora, comm-beta]
You need to log in before you can comment on or make changes to this bug.