Closed Bug 505056 Opened 11 years ago Closed 8 years ago

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

Categories

(SeaMonkey :: MailNews: General, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.13

People

(Reporter: InvisibleSmiley, Assigned: philip.chee)

References

(Depends on 1 open bug)

Details

Attachments

(2 files, 2 obsolete files)

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.
Blocks: TB2SM
No longer blocks: TB2SM
Depends on: 314124, 496429
Blocks: TB2SM
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: nobody → philip.chee
Status: NEW → ASSIGNED
Attached patch Patch v1.0 Proposed fix. (obsolete) — Splinter Review
> +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)
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)
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+
Blocks: 536390
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 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 ;-)
>>-          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?
> 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 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.
I haven't tested, but on Mac, GrayText equals to NSColor disabledControlTextColor. Not sure that's the right text color here.
>>+          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...
Attachment #639619 - Flags: review?(neil) → review+
Attachment #639619 - Flags: superreview?(mnyromyr)
(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.
(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 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+
Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/ad15062dae0b
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.13
No longer blocks: TB2SM
No longer blocks: 536390
Blocks: 404580
You need to log in before you can comment on or make changes to this bug.