#610 closed enhancement (fixed)
Journal Palette does not manage too many characters for a title correctly
Reported by: | IwikiwI | Owned by: | tomeu |
---|---|---|---|
Priority: | Normal | Milestone: | |
Component: | journal | Version: | Unspecified |
Severity: | Minor | Keywords: | right click journal sugar-love |
Cc: | eben, sascha_silbe, benzea | Distribution/OS: | Fedora |
Bug Status: | Assigned |
Description
When right clicking on a journal item, if the name happens to be n characters long (These n being beyond screen size) the menu extends beyond the screen, and no menu items are displayed.
I have a temporary fix:
if metadata.has_key('title'):
#Hack limits to specified number of characters, and saves screen from going all the way!
screen=gtk.gdk.Screen()
width = screen.get_width()
if width >= 800 and width <= 1200:
char_length = 15
elif width > 1200 and width <= 1650:
char_length = 20
else:
char_lenghth = 25
if len(metadatatitle?)>char_length:
title = metadatatitle?[0:char_length]+"..."
else:
title = metadatatitle?
any suggestions for a good permanent fix?
Attachments (1)
Change History (25)
comment:1 Changed 15 years ago by eben
comment:2 Changed 15 years ago by erikos
- Summary changed from right click on journal items to Journal Palette does not manage too many characters for a title correctly
comment:3 Changed 15 years ago by sascha_silbe
- Cc sascha_silbe added
comment:4 Changed 15 years ago by tomeu
- Severity changed from Minor to Blocker
comment:5 Changed 15 years ago by erikos
- Milestone changed from Unspecified by Release Team to 0.84
comment:6 Changed 15 years ago by erikos
- Bug Status changed from Unconfirmed to Assigned
comment:7 Changed 15 years ago by homunq
- Keywords sugar-love added
comment:8 Changed 15 years ago by tomeu
I don't see an unintrusive way to do this properly in 0.84, so unless someone speaks, I'm just going to do the following for 0.84:
diff --git a/src/sugar/graphics/palette.py b/src/sugar/graphics/palette.py index 919721b..1c668b6 100644 --- a/src/sugar/graphics/palette.py +++ b/src/sugar/graphics/palette.py @@ -145,7 +145,7 @@ class Palette(gtk.Window): # DEPRECATED: label is passed with the primary-text property, accel_path # is set via the invoker property, and menu_after_content is not used def __init__(self, label=None, accel_path=None, menu_after_content=False, - text_maxlen=0, **kwargs): + text_maxlen=60, **kwargs): self.palette_state = self.PRIMARY
comment:9 Changed 15 years ago by IwikiwI
Sorry, I was busy with my application and stuff.
I will get back to this. Can you give me 8 hrs?
comment:10 Changed 15 years ago by IwikiwI
Eben,
So here's what I am thinking
If say 10 characters are 20 pango units. And naturally 20 characters will definitely be above 20 pango units. So we can be sloppy, and ideally limit characters by pango units. I will see if theres a method to slice a string with respect to pango units. If not long way parse each character add up, and slice then.
And if the characters are around say 8-13 then we do a check on each character width, and perform a trim.
comment:11 Changed 15 years ago by IwikiwI
if text_maxlen > 0:
self._label.set_max_width_chars(text_maxlen)
self._label.set_ellipsize(pango.ELLIPSIZE_END)
Ok! There's already code built to slice the string when the string goes beyond the
I will just need to draw the palette frame according to the screen size, will this do?
comment:12 Changed 15 years ago by eben
I'm not sure this solves the problem. The root of the issue is that we have no way, presently, to trim the text label to a specific width. We should allow setting the min/max width/height of a palette, and adjust the strings within accordingly. The current hack allows one to trim the string to a specific number of characters, which is not what's really needed.
As a note, I think we should be using ELLIPSIZE_MIDDLE, so that we don't truncate the tail of the string where useful identifiers (such as the number of date of the document) often live.
comment:13 Changed 15 years ago by benzea
Hm, I am not sure if there is an ideal way to fix this ...
I first thought that the best way is to limit the size of the palette in do_size_request. This is rather simple, but one runs into the issue, that the label requests a way too small area if ellipsizing is enabled. Wich means you the palettes are also too small.
So how about:
- Limit the size to a certain maximum in do_size_request (make this value quite large, but eg. make sure that it is not larger than the screen). Keep in mind that a eg. a colorpicker palette may be quite large.
- Set max_width_chars to an insanely large value, and enable ellipsizing.
The first point makes sure the palette does not get too large, the second one enforces the label to try to allocate as much space is as needed. (If you do not set max_width_chars, then it will only request enough space to show the '...'.)
The corner case are still menu items with the names in them (eg. in the home view when starting the journal entry). Their size is also larger than the screen, but they may not have the hack (ie. point 2) in place. This means they will not be ellipsized, but instead the string is just truncated at some point.
comment:14 Changed 15 years ago by eben
The palette size shouldn't be limited by the palette class. This should be settable by the creator of the palette. For instance, palettes in the Journal, or in the home/groups/neighborhood views, should be a maximum of 1/2 the width of the screen, so that one edge of them can always align with the mouse cursor. On the other hand, palettes attached to toolbar buttons could be up to the full width of the screen. (This handles your color-picker case.)
Also, why shouldn't we just set max_width_chars to be a large value, with ellipsizing, in the palette class? The creator of the palette should indicate a maximum size for the palette, and the palette itself should ensure that the primary and secondary labels are treated accordingly, ellipsizing if needed.
comment:15 Changed 15 years ago by benzea
- Cc benzea added
comment:16 Changed 15 years ago by benzea
Attached a patch.
Instead of having the text_maxlen parameter, we could just have a maxwidth parameter, which could just be 0.5 if the maximum width should be half the screen size. It should be quite simple to change the patch in that way.
Another improvement would be to change menuitem.py to do the same text_maxwidth hack with a large max_width_chars value, and ellipsizing enabled by default.
comment:17 Changed 15 years ago by tomeu
Simple patch now limiting MenuItems as well:
diff --git a/src/sugar/graphics/menuitem.py b/src/sugar/graphics/menuitem.py index a357d78..7af8a6a 100644 --- a/src/sugar/graphics/menuitem.py +++ b/src/sugar/graphics/menuitem.py @@ -28,7 +28,7 @@ import gtk from sugar.graphics.icon import Icon class MenuItem(gtk.ImageMenuItem): - def __init__(self, text_label=None, icon_name=None, text_maxlen=0, + def __init__(self, text_label=None, icon_name=None, text_maxlen=60, xo_color=None, file_name=None): gobject.GObject.__init__(self) self._accelerator = None diff --git a/src/sugar/graphics/palette.py b/src/sugar/graphics/palette.py index 919721b..1c668b6 100644 --- a/src/sugar/graphics/palette.py +++ b/src/sugar/graphics/palette.py @@ -145,7 +145,7 @@ class Palette(gtk.Window): # DEPRECATED: label is passed with the primary-text property, accel_path # is set via the invoker property, and menu_after_content is not used def __init__(self, label=None, accel_path=None, menu_after_content=False, - text_maxlen=0, **kwargs): + text_maxlen=60, **kwargs): self.palette_state = self.PRIMARY
comment:18 Changed 15 years ago by tomeu
- Keywords r? added
comment:19 Changed 15 years ago by tomeu
- Milestone changed from 0.84 to 0.86
- Severity changed from Blocker to Minor
r+'ed by Simon in #sugar
fixed by http://git.sugarlabs.org/projects/sugar-toolkit/repos/mainline/commits/b219dff0
Moving to 0.86 in case people want to fix it in a better way
comment:20 Changed 15 years ago by tomeu
- Keywords r? removed
comment:21 Changed 14 years ago by tomeu
- Milestone changed from 0.86 to 0.88
comment:22 Changed 14 years ago by garycmartin
- Milestone changed from 0.88 to 0.90
- Resolution set to fixed
- Status changed from new to closed
Closing this ticket, if folks don't like this specific fix, a new ticket could be opened in the future.
comment:23 Changed 14 years ago by garycmartin
- Milestone changed from 0.90 to 0.88
This shouldn't be left to the palette creator to manage. In fact, I thought palettes already had some code which enforced a maximum width. In any case, the palette class is where the fix should be, I think.
Also, we're not using a fixed-width font so depending on a character limit isn't robust enough. We should be trimming to fit a specific width relative to the screen size.