Opened 12 years ago

Closed 12 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 12 years ago.
Possible fix
IconMenuItem.cpp.2.diff (1.8 KB) - added by jonas.kirilla 12 years ago.

Download all attachments as: .zip

Change History (8)

Changed 12 years ago by jonas.kirilla

Attachment: IconMenuItem.cpp.diff added

Possible fix

comment:1 Changed 12 years ago by axeld

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.

comment:2 in reply to:  1 Changed 12 years ago by jonas.kirilla

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 Changed 12 years ago by axeld

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

Changed 12 years ago by jonas.kirilla

Attachment: IconMenuItem.cpp.2.diff added

comment:4 Changed 12 years ago by jonas.kirilla

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.

comment:5 in reply to:  4 Changed 12 years ago by jonas.kirilla

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 Changed 12 years ago by axeld

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.