Port bug 314124 - Folder Pane Popup over folders with unseen messages

RESOLVED FIXED in seamonkey2.13

Status

SeaMonkey
MailNews: General
--
enhancement
RESOLVED FIXED
8 years ago
5 years ago

People

(Reporter: InvisibleSmiley, Assigned: Philip Chee)

Tracking

(Depends on: 1 bug)

Trunk
seamonkey2.13
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

8 years ago
TB bug 314124 implemented a tooltip showing information like sender, subject and preview for up to eight NEW messages of a folder (e.g. IMAP Inbox, RSS feed folder). SM already has the corresponding code in its mailWidgets.xml and bug 174234 will enable the tooltip (initially only to show full newsgroup names). What's left to do is porting the three mail.biff.alert.* prefs (and decide whether to activate them) and adding the styling through (theme-specific) CSS.

Note: mind bug 496429 for the prefs.
(Reporter)

Updated

8 years ago
Blocks: 360488
(Reporter)

Updated

8 years ago
No longer blocks: 360488
Depends on: 314124, 496429

Updated

8 years ago
Blocks: 360488

Comment 1

7 years ago
I understand that this bug is responsible for the request I am making...

-------- Original Message --------
Subject: Adjusting new email notification to give a teaser of the new message
Date: Tue, 30 Nov 2010 09:40:18 -0500
Newsgroups: mozilla.dev.apps.seamonkey

Greetings,

Again missing TB 2.x functionality. That code would show the message subject and teaser of arriving emails.

I have over 700 folders, many of which have unread mail in them.

Giving such information in the pop-up new message indicator would expedite if I need to attend to the new message immediately or not.

So, could that code get borrowed from TB 2.x please? Thanks!
(Assignee)

Updated

5 years ago
Assignee: nobody → philip.chee
Status: NEW → ASSIGNED
(Assignee)

Comment 2

5 years ago
Created attachment 636161 [details] [diff] [review]
Patch v1.0 Proposed fix.

> +pref("mail.biff.alert.show_preview", true);
> +pref("mail.biff.alert.show_subject", true);
> +pref("mail.biff.alert.show_sender",  true);
> +pref("mail.biff.alert.preview_length", 40);

These preferences should eventually be moved to shared /mailnews and removed from Thunderbird but not until we get the flying mail toasters.

> +.folderSummary-previewText {
> +/*
> +  color: grey;          #808080
> +         Indigo;        #4B0082
> +         DimGray;       #696969
> +         DarkSlateGray; #2F4F4F
> +*/
> +  color: DimGray;

Thunderbird uses Grey/Gray but I think it's too light. I'm rather partial to Indigo, but I expect that's just me. I think DimGray is dark enough to be read without eye strain.


> +.folderSummary-previewText {
> +  color: #696969; /* DimGray; */
> +}
Either way up/Your place or mine!
Attachment #636161 - Flags: ui-review?(stefanh)
Attachment #636161 - Flags: review?(neil)
(Assignee)

Comment 3

5 years ago
Comment on attachment 636161 [details] [diff] [review]
Patch v1.0 Proposed fix.

Jens did most of the groundwork in Bug 174234, so asking him for feedback.
Attachment #636161 - Flags: feedback?(jh)
(Reporter)

Comment 4

5 years ago
Comment on attachment 636161 [details] [diff] [review]
Patch v1.0 Proposed fix.

Folder popup works, colors/contrast are fine with me, traditional "toaster" biff popups still work. f=me (only tested on Win7).

You should probably add some comment accompanying the new prefs to explain we don't have full support yet and want to move them to mailnews/ eventually.

Possible follow-ups:
* sync the new prefs (add a comment/dependency to bug 728949)
* add to Preferences (new bug)
* adapt Help (new bug)
Attachment #636161 - Flags: feedback?(jh) → feedback+
(Assignee)

Updated

5 years ago
Blocks: 536390

Comment 5

5 years ago
Comment on attachment 636161 [details] [diff] [review]
Patch v1.0 Proposed fix.

Looks good on Mac (10.7). I agree that DimGray looks better than gray
Attachment #636161 - Flags: ui-review?(stefanh) → ui-review+

Comment 6

5 years ago
Comment on attachment 636161 [details] [diff] [review]
Patch v1.0 Proposed fix.

>-          var popupValue = null;
>+          var popupValue;
Nit: unnecessary.

>-            let tooltip = document.createElementNS("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul", "label");
>-            tooltip.setAttribute("value", popupValue);
>-            tooltip.className = "tooltip-label";
>+            let loc = document.createElementNS("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul", "folderSummaryLocation");
>+            loc.setAttribute("location", popupValue);
>             let folderSummary = document.getAnonymousNodes(this)[0];
>-            document.getAnonymousNodes(folderSummary)[0].appendChild(tooltip);
>+            document.getAnonymousNodes(folderSummary)[0].appendChild(loc);
Why this change?

>+.folderSummary-previewText {
>+  color: DimGray;
Had you asked me I would have suggested GrayText for Classic and #8C99AB for Modern.
I guess I'd better try them out to see what they look like ;-)
(Assignee)

Comment 7

5 years ago
Created attachment 639368 [details]
Screenshot of folder name as tooltip + summaries.

>>-          var popupValue = null;
>>+          var popupValue;
> Nit: unnecessary.
Fixed locally.

>>-            let tooltip = document.createElementNS("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul", "label");
>>-            tooltip.setAttribute("value", popupValue);
>>-            tooltip.className = "tooltip-label";
>>+            let loc = document.createElementNS("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul", "folderSummaryLocation");
>>+            loc.setAttribute("location", popupValue);
>>             let folderSummary = document.getAnonymousNodes(this)[0];
>>-            document.getAnonymousNodes(folderSummary)[0].appendChild(tooltip);
>>+            document.getAnonymousNodes(folderSummary)[0].appendChild(loc);
> Why this change?
Hmm. I've reverted the change and retested. The tooltip doesn't align with the folder summaries, unless I remove the "tooltip-label" classname.

>>+.folderSummary-previewText {
>>+  color: DimGray;
> Had you asked me I would have suggested GrayText for Classic and #8C99AB for Modern.
> I guess I'd better try them out to see what they look like ;-)
And?
(Assignee)

Comment 8

5 years ago
Created attachment 639385 [details] [diff] [review]
Patch v1.1 back to using a label but without .tooltip-label class

> Hmm. I've reverted the change and retested. The tooltip doesn't align with the
> folder summaries, unless I remove the "tooltip-label" classname.
Attachment #636161 - Attachment is obsolete: true
Attachment #636161 - Flags: review?(neil)
Attachment #639385 - Flags: review?(neil)

Comment 9

5 years ago
Comment on attachment 639385 [details] [diff] [review]
Patch v1.1 back to using a label but without .tooltip-label class

>+          return !!popupValue || newMessages;
Does this not work without the !! ?

>+  <binding id="folderSummary-location">
Do we still need this?

>+folderSummaryLocation
Or this?

>+.folderSummary-previewText {
>+  color: #696969; /* DimGray; */
>+}
As I recall, I liked my colours. Sorry for mentioning it before.

Comment 10

5 years ago
I haven't tested, but on Mac, GrayText equals to NSColor disabledControlTextColor. Not sure that's the right text color here.
(Assignee)

Comment 11

5 years ago
Created attachment 639619 [details] [diff] [review]
Patch v1.2 Fix Nits.

>>+          return !!popupValue || newMessages;
> Does this not work without the !! ?
Hmm. So it does.

>>+  <binding id="folderSummary-location">
> Do we still need this?
No. reverted.

>>+folderSummaryLocation
> Or this?
No. reverted.

>>+.folderSummary-previewText {
>>+  color: #696969; /* DimGray; */
>>+}
> As I recall, I liked my colours. Sorry for mentioning it before.
Fixed.

> Had you asked me I would have suggested GrayText for Classic and #8C99AB for Modern.
> I guess I'd better try them out to see what they look like ;-)

> Stefan [:stefanh]      2012-07-05 12:59:31 PDT
> 
> I haven't tested, but on Mac, GrayText equals to NSColor disabledControlTextColor.
> Not sure that's the right text color here.
Suggestions needed.
Attachment #639385 - Attachment is obsolete: true
Attachment #639385 - Flags: review?(neil)
Attachment #639619 - Flags: review?(neil)
(In reply to Stefan from comment #10)
> Not sure that's the right text color here.

.menulist-description uses GrayText too...

Updated

5 years ago
Attachment #639619 - Flags: review?(neil) → review+
(Assignee)

Updated

5 years ago
Attachment #639619 - Flags: superreview?(mnyromyr)

Comment 13

5 years ago
(In reply to neil@parkwaycc.co.uk from comment #12)
> (In reply to Stefan from comment #10)
> > Not sure that's the right text color here.
> 
> .menulist-description uses GrayText too...

Sure, but the tooltip background is yellow. I'll test this later on today, then we'll see.

Comment 14

5 years ago
(In reply to Stefan [:stefanh] from comment #13)
> Sure, but the tooltip background is yellow. I'll test this later on today,
> then we'll see.

Turns out that GrayText is 7F7F7F on Mac. I expected the differences to be much more visible, but in reality, on the yellow background, the differences between the colors are minor. Gray and GrayText looks about the same, DimGray is imo a bit better to the eye - but GrayText or Gray looks ok too.

Comment 15

5 years ago
Comment on attachment 639619 [details] [diff] [review]
Patch v1.2 Fix Nits.

Cool! 
(It only took me a while to realize that I need new stuff in the folder to cause the popup. ^_^)
Attachment #639619 - Flags: superreview?(mnyromyr) → superreview+
(Assignee)

Comment 16

5 years ago
Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/ad15062dae0b
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.13
(Assignee)

Updated

5 years ago
No longer blocks: 360488
(Assignee)

Updated

5 years ago
No longer blocks: 536390
(Assignee)

Updated

5 years ago
Blocks: 404580
You need to log in before you can comment on or make changes to this bug.