Opened 13 years ago

Closed 13 years ago

#1033 closed bug (fixed)

8-bit icons in Tracker's context menu's "New" submenu

Reported by: jonas.kirilla Owned by: axeld
Priority: normal Milestone: R1
Component: Applications/Tracker Version: R1/pre-alpha1
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: no Platform: All

Description

Tracker's context menu's "New" menu items still show 8-bit icons, so here's a patch for review and commit:

http://www.kirilla.com/haiku/IconMenuItem.cpp.diff

I don't know if the Push/PopState() is necessary.

Attachments (2)

IconMenuItem.cpp.diff (1.7 KB ) - added by jonas.kirilla 13 years ago.
Possible fix
IconMenuItem.cpp.2.diff (1.8 KB ) - added by jonas.kirilla 13 years ago.

Download all attachments as: .zip

Change History (8)

by jonas.kirilla, 13 years ago

Attachment: IconMenuItem.cpp.diff added

Possible fix

comment:1 by axeld, 13 years ago

Thanks for the patch! Bug reports like this are the best :-) Unfortunately, I can't commit it, as it will break Tracker's "New" menu on R5. If you have the time to rework it for both builds, I'd appreciate it, but if not, I can do it as well. Just tell me.

The Push/PopState() should not be necessary (at least not if it wasn't before), but it shouldn't hurt either.

in reply to:  1 comment:2 by jonas.kirilla, 13 years ago

Replying to axeld:

If you have the time to rework it for both builds, I'd appreciate it, but if not, I can do it as well. Just tell me.

I can do it tomorrow or early next week. I suppose with ifdefs for Haiku/BeOS in the IconMenuItems' constructors bitmap creation (CMAP8/RGB32) and for part of DrawContent().

comment:3 by axeld, 13 years ago

That should pretty much be it, yes :-) Thanks!

by jonas.kirilla, 13 years ago

Attachment: IconMenuItem.cpp.2.diff added

comment:4 by jonas.kirilla, 13 years ago

I think I'm done. See attachment: IconMenuItem.cpp.2.diff

Axel, feel free to edit as you see fit. I'll take a look at your changes in SVN.

in reply to:  4 comment:5 by jonas.kirilla, 13 years ago

Having had another look at the result, magnified, I think the line "where.y = Frame().top + 2;" should be "where.y = Frame().top + 1;" instead, so there's space below as well as above the icon.

comment:6 by axeld, 13 years ago

Resolution: fixed
Status: newclosed

Thanks! Patch applied in hrev20240. While I was at it, I've changed the positioning code to duplicate the method used in ModelMenuItem which should work better with larger font sizes.

Note: See TracTickets for help on using tickets.