Closed
Bug 956589
Opened 10 years ago
Closed 10 years ago
Port Bug 954653 - Redesign buddy tooltips
Categories
(Thunderbird :: Instant Messaging, defect)
Thunderbird
Instant Messaging
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 29.0
People
(Reporter: Paenglab, Assigned: Paenglab)
References
Details
Attachments
(1 file, 2 obsolete files)
11.76 KB,
patch
|
aleth
:
review+
|
Details | Diff | Splinter Review |
This bug is to follow the IB tooltip redesign form bug 954653.
Assignee | ||
Comment 1•10 years ago
|
||
This patch needs bug 956579 applied first.
Comment 2•10 years ago
|
||
(In reply to Richard Marti (:Paenglab) from comment #1) > This patch needs bug 956579 applied first. After both these patches are applied, how forked is buddytooltip.xml/imbuddytooltip.xml, really? (Other than the path)
Assignee | ||
Comment 3•10 years ago
|
||
(In reply to aleth from comment #2) > > After both these patches are applied, how forked is > buddytooltip.xml/imbuddytooltip.xml, really? (Other than the path) By porting this bug, I saw buddiesElt is missing in imbuddytooltip.xml: @@ -267,17 +282,17 @@ let buddies = this.contact.getBuddies(); if (buddies.length <= 1) return true; let buddiesElt = document.getAnonymousElementByAttribute(this, "anonid", "buddies"); let sep = document.createElement("separator"); - sep.setAttribute("class", "groove"); + sep.setAttribute("class", "groove tooltipSeparator"); buddiesElt.appendChild(sep); for each (let buddy in buddies) { let buddyElt = document.createElement("buddy"); buddiesElt.appendChild(buddyElt); buddyElt.build(buddy, this); }
Comment 4•10 years ago
|
||
Comment on attachment 8355914 [details] [diff] [review] patch Review of attachment 8355914 [details] [diff] [review]: ----------------------------------------------------------------- It would probably have been better to merge patches in the order in which they landed in Instantbird: some of the changes of bug 956589 were included in the patch for bug 956579. >By porting this bug, I saw buddiesElt is missing in imbuddytooltip.xml: This is due to updateTooltipFromContact being much simpler for TB, as TB contacts don't ever contain multiple buddies at the moment. >diff --git a/chat/themes/buddytooltip.css b/chat/themes/buddytooltip.css >new file mode 100644 >--- /dev/null >+++ b/chat/themes/buddytooltip.css This new file should live in mail/components/im/content. While we should certainly try to unfork the tooltip code asap, until that is done, this file shouldn't be in /chat. Looks good otherwise!
Attachment #8355914 -
Flags: review?(aleth) → review-
Assignee | ||
Comment 5•10 years ago
|
||
(In reply to aleth from comment #4) > Comment on attachment 8355914 [details] [diff] [review] > patch > > Review of attachment 8355914 [details] [diff] [review]: > ----------------------------------------------------------------- > > It would probably have been better to merge patches in the order in which > they landed in Instantbird: some of the changes of bug 956589 were included > in the patch for bug 956579. Yeah, I saw this also after putting the patch for bug 956579 for review. > >diff --git a/chat/themes/buddytooltip.css b/chat/themes/buddytooltip.css > >new file mode 100644 > >--- /dev/null > >+++ b/chat/themes/buddytooltip.css > > This new file should live in mail/components/im/content. While we should > certainly try to unfork the tooltip code asap, until that is done, this file > shouldn't be in /chat. Moved.
Attachment #8355914 -
Attachment is obsolete: true
Attachment #8359304 -
Flags: review?(aleth)
Comment 6•10 years ago
|
||
Comment on attachment 8359304 [details] [diff] [review] patch v2 >diff --git a/mail/components/im/content/imBuddytooltip.css b/mail/components/im/content/imBuddytooltip.css >new file mode 100644 >--- /dev/null >+++ b/mail/components/im/content/imBuddytooltip.css nit: This should be imbuddytooltip.css to be consistent with imbuddytooltip.xml.
Attachment #8359304 -
Flags: review?(aleth) → review-
Assignee | ||
Comment 7•10 years ago
|
||
Now it's inconsistent with chrome://messenger/skin/imBuddytooltip.css ;)
Attachment #8359304 -
Attachment is obsolete: true
Attachment #8359704 -
Flags: review?(aleth)
Comment 8•10 years ago
|
||
Comment on attachment 8359704 [details] [diff] [review] patch v3 Review of attachment 8359704 [details] [diff] [review]: ----------------------------------------------------------------- (In reply to Richard Marti (:Paenglab) from comment #7) > Created attachment 8359704 [details] [diff] [review] > patch v3 > > Now it's inconsistent with chrome://messenger/skin/imBuddytooltip.css ;) This is true. Yet another reason to unfork this code ;) Thanks!
Attachment #8359704 -
Flags: review?(aleth) → review+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 9•10 years ago
|
||
https://hg.mozilla.org/comm-central/rev/76d9545cff20
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 29.0
You need to log in
before you can comment on or make changes to this bug.
Description
•