1388676 - [meta] Implement Photon bookmarks toolbar style

Status: RESOLVED FIXED

(Firefox :: Theme, enhancement, P3)

Description

[Johann Hofmann [:johannh]]

+++ This bug was initially created as a clone of Bug #1369415 +++

Spec is here: https://mozilla.invisionapp.com/share/MRBK1MZF7#/screens/237332831

Comment 1

[Johann Hofmann [:johannh]]

Attachment: Bug 1388676 - Consolidate bookmark toolbar styling across platforms to the new Photon design.

Review commit: https://reviewboard.mozilla.org/r/166498/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/166498/

Comment 2

[Johann Hofmann [:johannh]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e311022b53662c5cef4b4d2df74e92c43cddf025

Comment 3

[Dão Gottwald [:dao]]

Comment on attachment 8895302 [details]
Bug 1388676 - Consolidate bookmark toolbar styling across platforms to the new Photon design.

https://reviewboard.mozilla.org/r/166498/#review171646

See bug 1369415 comment 22 and my note from irc wrt to the spacing issue: we should properly explain and document such changes, if not out of respect for the community then for archeological purposes. Otherwise the next dev will come along not understanding the change and either revert without knowing the whole story, or be afraid to touch it at all, when instead we should enable them to make an informed decision.

I see no upside to lumping all these changes into a single patch. Breaking this up and properly explaining it is easy enough and not a waste of time.

Comment 4

[Dão Gottwald [:dao]]

Sorry for the noise, I filed bug 1388699 and bug 1388700 for the remaining missing pieces.

Comment 5

[Edo Karabas]

I believe there should be also a separate bug for a compact mode bookmarks toolbar.

Comment 6

[Dão Gottwald [:dao]]

(In reply to Eddward from comment #5)
> I believe there should be also a separate bug for a compact mode bookmarks
> toolbar.

Well spotted. We can take care of this in bug 1388700.

Comment 7

[Edo Karabas]

Yes, bug 1388700 should be about toolbar items density, but I'm also talking about bookmarks toolbar itself. Currently in Compact mode it is still too thick. As per Mockup should be 4px (I think) less height. Don't see a bug covering this. Probably similar, but opposite issue will be with a Touch mode.
See: https://s2.postimg.org/4eia9upbt/compact_bookmark_toolbar.png