470417 - Tidy up CSS after Bug 428216

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, enhancement)

Description

[Philip Chee]

Attachment: Patch v1.0 shuffle stuff

In order to make reviewing the patch in Bug 428216 easier I moved several styles to the ends of navigator.css. This patch moves the styles to a more reasonable position. In the attached patch I have also included a fix for Bug 470034.

Comment 1

[Philip Chee]

> Created an attachment (id=353808) [details]
> Patch v1.0 shuffle stuff

-/* Prevent [mode="icons"|"text"] from hiding the label and icon */
-#bookmarks-ptf .bookmark-item > .toolbarbutton-text,
-#bookmarks-ptf .bookmark-item > .toolbarbutton-icon {
+/* Prevent [mode="icons"] from hiding the label */
+#bookmarks-ptf .bookmark-item > .toolbarbutton-text {
   display: -moz-box !important;
+}

Initially I followed Firefox here. Historically XPFE the personal toolbar and the toolbar bookmark items were never configurable. The patch in Bug 428216 maintained this immutability. This patch allows the icon to be turned off/on according to the mode setting of the toolbar however the labels are still always forced on. The reasoning is as follows:

The bookmark items may never have any (fav)icons if the favicons and site_icons preferences are turned off (the default). Hiding the labels would just shows a row of identical "bookmark" icons and would probably cause some consternation.

turning off the icons is safe in that the bookmark items (should) always have distinguishable text.

Comment 2

[Philip Chee]

There is also a slight inconsistency in the "bookmarks-button" button that I haven't addressed yet and am unsure if it does need to be adjusted. This button has a class of "bookmark-item" rather than "toolbarbutton-1 chromeclass-toolbar-additional". This means that:

1. It is styled as a bookmark item with the label to the side of the icon, and
2. It does not respond to the toolbar mode and label position settings.

Should I change the class to toolbarbutton-1 so that it can react to the toolbar settings?

Comment 3

[[e-mail]]

Comment on attachment 353808 [details] [diff] [review]
Patch v1.0 shuffle stuff

r=me except on the bookmark icon changes - I think we need to be able to differentiate between folders and bookmarks.

Comment 4

[[e-mail]]

(In reply to comment #2)
> There is also a slight inconsistency in the "bookmarks-button" button that I
> haven't addressed yet and am unsure if it does need to be adjusted. This button
> has a class of "bookmark-item" rather than "toolbarbutton-1
> chromeclass-toolbar-additional".
Presumably this is to allow it to be dragged separately? I think we may have to add extra "fixup" rules like we do for the Home button in that case.

Comment 5

[Philip Chee]

Attachment: Patch v1.1 *only* shuffle stuff

Carrying forward r+, requesting sr+

Shuffle only changes, leaving out the bookmark item changes.

Comment 6

[Philip Chee]

Comment on attachment 353839 [details] [diff] [review]
Patch v1.1 *only* shuffle stuff

No UI changes so no sr required (I think)

Comment 7

[Stefan [:stefanh]]

Pushed changeset 7af00ea1e7c2 to comm-central.