Closed Bug 922834 Opened 11 years ago Closed 11 years ago

Australis: Need a more high-res version of the bookmarks toolbar item placeholder icon for the palette/panel

Categories

(Firefox :: Theme, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 29

People

(Reporter: Gijs, Assigned: Gijs)

References

Details

(Whiteboard: [Australis:P3])

Attachments

(4 files, 3 obsolete files)

The current icon isn't very big, because it was designed for use in toolbars. We should have a more high-resolution one, and/or decide if this icon fits with the other icons in the panel. Stephen, this probably needs your help (or that of someone else who can do the icon-wrangling for us). (Filed based on bug 878544 comment 4, but I noticed this myself when looking at bookmark toolbar items + panel stuff)
Flags: needinfo?(shorlander)
Whiteboard: [Australis:M?][Australis:P5] → [Australis:M?][Australis:P3]
Looks hi-res to me. Is this asset still needed?
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to mmaslaney from comment #1) > Created attachment 8345462 [details] > Screen Shot 2013-12-10 at 12.15.21 PM.png > > Looks hi-res to me. Is this asset still needed? That's the bookmarks menu button, not the bookmarks toolbar item.
Flags: needinfo?(gijskruitbosch+bugs)
Attached image bookmarksToolbar@2x.png (obsolete) —
Attachment #8345462 - Attachment is obsolete: true
Flags: needinfo?(shorlander)
2 things to note : This icon should also be updated in the "bookmarks sidebar" and the library. The text is cropped a bit weirdly in customization mode. It should appear on two lines.
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
(In reply to Guillaume C. [:ge3k0s] from comment #5) > 2 things to note : > > This icon should also be updated in the "bookmarks sidebar" and the library. Why? The icon isn't changing outside the panel/palette. It'll be the same in toolbars as well. This bug is purely out of concern for the resolution of the icon when it's displayed at 36px^2. Certainly changing the icon everywhere doesn't need to happen as part of this patch. Furthermore, the flat icon will look out of place in the current bookmarks UI, so we'd have to refresh all of those icons, which is even more out of scope here.
So this patch does fix the alignment of the icon by making palette wrappers remove and re-add flex for the things they wrap. I chose not to fix the text wrapping in here because we're still figuring out what the desired endstate is for cropped single-line vs. multiline labels in bug 944947.
Attachment #8360417 - Flags: review?(mconley)
Flags: needinfo?(mmaslaney)
Updated with all platform themes.
Flags: needinfo?(mmaslaney)
Comment on attachment 8360417 [details] [diff] [review] Australis: Need a more high-res version of the bookmarks toolbar item placeholder icon for the palette/panel, Cancelling review because I should integrate the other icons for Windows.
Attachment #8360417 - Flags: review?(mconley)
So this adds the WinXP icon. I'm going to leave the win8 bit for Mike de Boer in bug 859751.
Attachment #8360692 - Flags: review?(mconley)
Attachment #8360417 - Attachment is obsolete: true
Comment on attachment 8360692 [details] [diff] [review] Australis: Need a more high-res version of the bookmarks toolbar item placeholder icon for the palette/panel, Review of attachment 8360692 [details] [diff] [review]: ----------------------------------------------------------------- Other than the little bug I found, I think this looks OK to me. Certainly better than what we had before. ::: browser/components/customizableui/src/CustomizeMode.jsm @@ +685,5 @@ > toolbarItem.checked = true; > } > > + if (aWrapper.hasAttribute("flex") && !toolbarItem.hasAttribute("flex")) { > + toolbarItem.setAttribute(aWrapper.getAttribute("flex")); This needs to be: toolbarItem.setAttribute("flex", aWrapper.getAttribute("flex"))
Attachment #8360692 - Flags: review?(mconley) → review+
With my stupidity addressed (at least as far as this patch is concerned), remote: https://hg.mozilla.org/integration/fx-team/rev/4a5c84a4d166
Whiteboard: [Australis:M?][Australis:P3] → [Australis:P3][fixed-in-fx-team]
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P3][fixed-in-fx-team] → [Australis:P3]
Target Milestone: --- → Firefox 29
Depends on: 964217
So this fixes the styling of the search bar and the bookmarks bar. There's still a separate issue that only occurs if moving the bookmarks toolbar items from the palette to the menu panel, but stops as soon as they've been in the toolbar. Don't think that needs to hold up this bug, though.
Attachment #8366666 - Flags: review?(mdeboer)
Comment on attachment 8366666 [details] [diff] [review] Backed out changeset 4a5c84a4d166 () Sigh. Wrong bug! :-)
Attachment #8366666 - Flags: review?(mdeboer)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: