-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Icon: deprecate icon_size #4
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,6 +32,7 @@ | |
from gi.repository import Rsvg | ||
import cairo | ||
|
||
from sugar3.graphics import style | ||
from sugar3.graphics.xocolor import XoColor | ||
from sugar3.util import LRU | ||
|
||
|
@@ -344,6 +345,25 @@ def __init__(self, **kwargs): | |
self._alpha = 1.0 | ||
self._scale = 1.0 | ||
|
||
if 'pixel_size' not in kwargs: | ||
kwargs['pixel_size'] = style.STANDARD_ICON_SIZE | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can also be done with a default argument value. Reviewer choose :) |
||
|
||
#FIXME: deprecate icon_size | ||
if 'icon_size' in kwargs: | ||
logging.warning("icon_size is deprecated. Use pixel_size instead.") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we add a wiki page for deprecations, then the URL should be mentioned in this warning. "See http..." |
||
logging.debug(kwargs['icon_size']) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I left this debug line accidentally. Please remove before pushing. |
||
|
||
menu_sizes = (Gtk.IconSize.MENU, Gtk.IconSize.DND, | ||
Gtk.IconSize.SMALL_TOOLBAR, Gtk.IconSize.BUTTON) | ||
|
||
if kwargs['icon_size'] in menu_sizes: | ||
kwargs['pixel_size'] = style.MENU_ICON_SIZE | ||
|
||
elif kwargs['icon_size'] == Gtk.IconSize.LARGE_TOOLBAR: | ||
kwargs['pixel_size'] = style.STANDARD_ICON_SIZE | ||
|
||
kwargs.pop('icon_size') | ||
|
||
GObject.GObject.__init__(self, **kwargs) | ||
|
||
def get_file(self): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -114,6 +114,7 @@ def zoom(units): | |
MEDIUM_ICON_SIZE = zoom(55 * 1.5) | ||
LARGE_ICON_SIZE = zoom(55 * 2.0) | ||
XLARGE_ICON_SIZE = zoom(55 * 2.75) | ||
MENU_ICON_SIZE = zoom(33) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Gtk.IconSize.MENU seems to be used in palettes, but PaletteMenuItem is using SMALL_ICON_SIZE. Maybe there is a difference that I'm missing, just wanted to point that out. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah. We can look at that later. The current change is needed to keep the sizes the same. In artwork for the theme at 100% we have icon sizes of 33px, and there is no 33px size in style.py .
|
||
|
||
settings = Gio.Settings('org.sugarlabs.font') | ||
FONT_SIZE = settings.get_double('default-size') | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
icon_size and pixel_size are gtk.Image properties, so they can also set with icon.props.icon_size =. What about handling this in
_sync_image_properties
? It seems like we are already calculating width/height from the properties there, and only there.If we want a default_pixel size, I suspect the best way is to pass it to the GObject
__init__
so that it's handled like a normal gobject property.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @dnarvaez,
good point. Here is another PR:
#5