293207 - Icons do not show up in menulist in classic theme and menulist is not wide enough in classic/modern theme

Status: RESOLVED FIXED

(SeaMonkey :: Themes, defect)

Description

[Ian Neal]

In menulist (for example preferences, navigator, internet search pane) items do
not have icons in classic theme but do in modern theme.

The menulist is also not wide enough in either theme so the selected item is
truncated (look at how AskJeeves appears - AskJee...). Seems to ignore the icon
in its calculation.

Comment 1

[Ian Neal]

Attachment: One possible way of fixing the issue v0.1

This patch:
* Adds an attribute iconic which can be used to mark menulists as containing icons
* When iconic="true" in classic theme show icon with display: -moz-box

Comment 2

[Ian Neal]

Attachment: The 16px bookmarks option v0.2

This patch:
* Moves the limiting of the icon size of bookmarks from menu.css to the bookmarksToolbar.css which means menu.css no longer needs the display:none entry.

Comment 3

[Ian Neal]

Attachment: The 16px bookmarks with radio/checkbox v0.2a

Changes since v0.2:
* Also fixed OS/2 version of menu.css
* Fixed menuitems of type checkbox and radio so that they indent properly when not checked.

Comment 4

[Ian Neal]

Attachment: Reduced 16px bookmarks with radio/checkbox v0.2b

Changes since v0.2a:
* Dropped list-style-image: inherit and height isn't required for indenting.

Comment 5

[Karsten Düsterloh]

Comment on attachment 235326 [details] [diff] [review]
Reduced 16px bookmarks with radio/checkbox v0.2b

I'd prefer the 0.2 approach. Adding an attribute for something that can be/is controlled by CSS anyway, to fix just one menu, isn't worth it, imo.


>Index: global/mac/menu.css
>===================================================================

No need to change this for Mac, it does already show icons in menuitems.
The cut-off menulist value, though, is still cut off...

>Index: global/win/menu.css
>===================================================================

The Windows menulist popup now shows the icons, but they're not quite right aligned: too much space to the left of the icon, too few on its right side.

Comment 6

[Andy Willis (abwillis)]

Prior to applying the patch on OS/2 we had an icon for the selected option but none in the drop down.  After applying the patch we do have the icons in the drop down and I have not seen any detrimental effects.

Comment 7

[Ian Neal]

Attachment: Classic theme patch with better icon spacing v0.2c

Changes since v0.2b:
* Fixed mac theme so menulist is not truncated
* Fixed win theme so icon has better spacing round it

Comment 8

[Ian Neal]

Attachment: Modern theme patch with wider menulist v0.3

This patch:
* Pads certain menulists less on the left
* Pads certain menulists slightly more on the right

Comment 9

[Karsten Düsterloh]

Comment on attachment 236469 [details] [diff] [review]
Modern theme patch with wider menulist v0.3

Looks okay on Win and Mac.

Comment 10

[Karsten Düsterloh]

Comment on attachment 236468 [details] [diff] [review]
Classic theme patch with better icon spacing v0.2c

Looks okay on Win and Mac.

Comment 11

[[e-mail]]

Comment on attachment 236468 [details] [diff] [review]
Classic theme patch with better icon spacing v0.2c

>-  padding: 1px 30px 1px 5px;
>+  padding: 1px 26px 1px 1px;
This padding could probably be trimmed a bit more, I got it down to 22px.
I can't comment on the other OSes, of course.

Comment 12

[[e-mail]]

Comment on attachment 236469 [details] [diff] [review]
Modern theme patch with wider menulist v0.3

I don't like the effect this has; previously the padding used to be equal on both sides of the menulist label. Maybe you can improve this by setting a minimum width on the image? Also, the editor image properties appearance tab doesn't work properly unlike the way you fixed it for Classic.

Comment 13

[Ian Neal]

Attachment: Classic theme patch with 22px v0.2d (Checked into trunk)

Patch that was checked into trunk for classic theme - 26px changed to 22px since v0.2c (if * { -moz-appearance: none !important; } is not set in userChrome.css could be set to 20px).

Comment 14

[Ian Neal]

Comment on attachment 238698 [details] [diff] [review]
Classic theme patch with 22px v0.2d (Checked into trunk)

Checking in (trunk)
communicator/bookmarks/bookmarksToolbar.css;
new revision: 1.24; previous revision: 1.23
global/mac/menu.css;
new revision: 1.44; previous revision: 1.43
global/os2/menu.css;
new revision: 1.3; previous revision: 1.2
global/win/menu.css;
new revision: 1.60; previous revision: 1.59
done

Comment 15

[Ian Neal]

Attachment: Modern theme patch with min-width/height v0.3a (Checked into trunk)

Changes since v0.3:
* Removed change to menulist.css
* Set a min-width/height for "icons" but fixed it for bookmark "icons"
* Increased from 28px to 30px to fix linux/non-native width issue

Comment 16

[[e-mail]]

Comment on attachment 238783 [details] [diff] [review]
Modern theme patch with min-width/height v0.3a (Checked into trunk)

Ingenious :-)

Comment 17

[Karsten Düsterloh]

Comment on attachment 238783 [details] [diff] [review]
Modern theme patch with min-width/height v0.3a (Checked into trunk)

Looks good on Mac and Windows. My apologies for the overlong delay. :-|

Comment 18

[Ian Neal]

Comment on attachment 238783 [details] [diff] [review]
Modern theme patch with min-width/height v0.3a (Checked into trunk)

Checking in (trunk)
communicator/bookmarks/bookmarksToolbar.css;
new revision: 1.15; previous revision: 1.14
global/menu.css;
new revision: 1.50; previous revision: 1.49
done