Closed Bug 1375893 Opened 7 years ago Closed 7 years ago

Update pinned-tab highlight/glow style for Photon

Categories

(Firefox :: Theme, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 56
Iteration:
56.2 - Jul 10
Tracking Status
firefox56 --- verified disabled
firefox57 --- verified

People

(Reporter: Dolske, Assigned: Dolske)

References

(Blocks 1 open bug)

Details

(Whiteboard: [photon-visual][p2])

Attachments

(2 files, 1 obsolete file)

The current highlight/glow on pinned tabs (used as a notification to indicate when the title has changed) doesn't work so well with the dark titlebar from bug 1367384 / bug 1367385. The Photon mocks show this as changing to a small blue dot.
Whiteboard: [photon-visual] → [photon-visual] [triage]
Blocks: photon-tabs
No longer blocks: 1349555
Flags: qe-verify+
Priority: -- → P2
Whiteboard: [photon-visual] [triage] → [photon-visual][p2]
QA Contact: brindusa.tot
Attached patch Patch v.1 (obsolete) — Splinter Review
I checked with Stephen -- this is the SVG from his demo mockup, and is fine to use. And the single image/color is fine for the default/light/dark themes.
Assignee: nobody → dolske
Attachment #8882094 - Flags: review?(dao+bmo)
Status: NEW → ASSIGNED
Priority: P2 → P1
Comment on attachment 8882094 [details] [diff] [review]
Patch v.1

>--- a/browser/themes/shared/compacttheme.inc.css
>+++ b/browser/themes/shared/compacttheme.inc.css

>+%ifdef MOZ_PHOTON_THEME
>+  --pinned-tab-glow-url: url(chrome://browser/skin/tabbrowser/indicator-tab-attention.svg);

>@@ -84,7 +88,11 @@ toolbar:-moz-lwtheme-brighttext  {
>   --tab-selection-color: #f5f7fa;
>   --tab-selection-background-color: #4c9ed9;
>   --tab-selection-box-shadow: none;
>+%ifdef MOZ_PHOTON_THEME
>+  --pinned-tab-glow-url: url(chrome://browser/skin/tabbrowser/indicator-tab-attention.svg);

> .tabbrowser-tab:-moz-any([image], [pinned]) > .tab-stack > .tab-content[attention]:not([selected="true"]),
> .tabbrowser-tab > .tab-stack > .tab-content[pinned][titlechanged]:not([selected="true"]) {
>+%ifdef MOZ_PHOTON_THEME
>+  background-image: var(--pinned-tab-glow-url);
>+  background-position: center bottom -4px;

Please remove background-image and the --pinned-tab-glow-url variable since you're not changing the value from what you already set in tabs.inc.css.
Attachment #8882094 - Flags: review?(dao+bmo) → review+
Pushed by jdolske@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b8e1f6c6ebd6
Update pinned-tab highlight/glow style for Photon. r=dao
https://hg.mozilla.org/mozilla-central/rev/b8e1f6c6ebd6
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Iteration: --- → 56.2 - Jul 10
Depends on: 1379052
(In reply to Justin Dolske [:Dolske] from comment #0)
> The Photon mocks show this as changing to a small blue dot.

This change introduced huge notification background for non-pinned tabs. Is it by purpose? 

See screenshot here: https://bug1383312.bmoattachments.org/attachment.cgi?id=8888959
See Also: → 1383312
No longer depends on: 1383449
QA Contact: brindusa.tot → ovidiu.boca
User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20170907220212

I would like to verify this issue but I don't know exactly what to follow or what to look for. I've opened Facebook, Facebook Messenger, Skype in the browser (tabs unpinned) but there is no notification or glow. Only the number of notification is displayed in front of the tab title. Is this the expected result?

I have used the Light theme with Compact density.
Flags: needinfo?(dolske)
For the pinned tabs the blue dot that represents the notification is displayed but not fully displayed. The bottom part is cut off. Is this also intended?
User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20170912220343

Verified on Windows 8.1 x64, on Windows 10 x64 and x32, Mac OS 10.12 and Ubuntu 14.04 with latest Nightly 57.0a1 and I cannot reproduce the issue.
Based on previous comment mark this bug verified.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
(In reply to Vlad Bacia-Mociran [:VladB] from comment #9)
> Created attachment 8905925 [details]
> pin notification cut off
> 
> For the pinned tabs the blue dot that represents the notification is
> displayed but not fully displayed. The bottom part is cut off. Is this also
> intended?

Yes.

(In reply to Vlad Bacia-Mociran [:VladB] from comment #8)

> I would like to verify this issue but I don't know exactly what to follow or
> what to look for. I've opened Facebook, Facebook Messenger, Skype in the
> browser (tabs unpinned) but there is no notification or glow. Only the
> number of notification is displayed in front of the tab title. Is this the
> expected result?

The tab's title text needs to change while it's in the background (since this highlight is meant to indicate unseen changes). IIRC when writing this I just used a little test page that did a setTimeout (to allow me to switch to another page) then then updated the title.

I see this every day in normal usage, so agree it's verified.
Flags: needinfo?(dolske)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: