Closed
Bug 505056
Opened 15 years ago
Closed 12 years ago
Port bug 314124 - Folder Pane Popup over folders with unseen messages
Categories
(SeaMonkey :: MailNews: General, enhancement)
SeaMonkey
MailNews: General
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)
(deleted),
image/png
|
Details | |
(deleted),
patch
|
neil
:
review+
mnyromyr
:
superreview+
|
Details | Diff | Splinter Review |
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.
Updated•15 years ago
|
Comment 1•14 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•12 years ago
|
Assignee: nobody → philip.chee
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•12 years ago
|
||
> +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•12 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•12 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+
Comment 5•12 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•12 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•12 years ago
|
||
>>- 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•12 years ago
|
||
> 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•12 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•12 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•12 years ago
|
||
>>+ 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)
Comment 12•12 years ago
|
||
(In reply to Stefan from comment #10)
> Not sure that's the right text color here.
.menulist-description uses GrayText too...
Updated•12 years ago
|
Attachment #639619 -
Flags: review?(neil) → review+
Assignee | ||
Updated•12 years ago
|
Attachment #639619 -
Flags: superreview?(mnyromyr)
Comment 13•12 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•12 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•12 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•12 years ago
|
||
Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/ad15062dae0b
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.13
You need to log in
before you can comment on or make changes to this bug.
Description
•