Closed Bug 470417 Opened 13 years ago Closed 13 years ago

Tidy up CSS after Bug 428216

Categories

(SeaMonkey :: UI Design, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.0a3

People

(Reporter: philip.chee, Assigned: philip.chee)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch Patch v1.0 shuffle stuff (obsolete) — Splinter Review
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.
Attachment #353808 - Flags: review?(neil)
> 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.
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 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.
Attachment #353808 - Flags: review?(neil) → review+
(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.
Carrying forward r+, requesting sr+

Shuffle only changes, leaving out the bookmark item changes.
Attachment #353808 - Attachment is obsolete: true
Attachment #353839 - Flags: superreview?(neil)
Attachment #353839 - Flags: review+
Comment on attachment 353839 [details] [diff] [review]
Patch v1.1 *only* shuffle stuff

No UI changes so no sr required (I think)
Attachment #353839 - Flags: superreview?(neil)
Pushed changeset 7af00ea1e7c2 to comm-central.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.0a3
Blocks: 459509
You need to log in before you can comment on or make changes to this bug.