After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 59799 - GtkEntry should permit right-aligned text
GtkEntry should permit right-aligned text
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: Other
1.3.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
: 68475 135177 (view as bug list)
Depends on:
Blocks: 72948
 
 
Reported: 2001-08-30 23:33 UTC by Christian Reis (not reading bugmail)
Modified: 2011-02-04 16:12 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Enables adjustment of text in GtkEntry (right, left and center) (2.97 KB, patch)
2002-12-07 09:54 UTC, ean
none Details | Review
Enables any alignment of text in GtkEntry (2.66 KB, patch)
2002-12-08 11:04 UTC, ean
none Details | Review
patch for gtkentry for having text aligned, includes changes according to Owen's comments (4.89 KB, patch)
2004-03-01 02:32 UTC, Steffen Gutmann
none Details | Review

Description Christian Reis (not reading bugmail) 2001-08-30 23:33:01 UTC
The 1.2 GtkEntry - and, as per Havoc's description, 1.3 is included -
should permit permit right-aligning the text inserted into it. This hampers
developers that strive to provide a consistent interface for numeric entry,
as numbers are custumarily entered aligned to the right. I would like to
point out that this is the current behaviour in GAL's e-text widget, and
that it is very satisfactory.

The current situation with 1.3 is, if the text direction is right-to-left,
that the GtkEntry aligns text to the right. This behaviour could be
extended to include left-to-right locales by allowing one to call
set_text_alignment()  on the GtkEntry. Other interfaces that offered
compatible functionality would be acceptable.
Comment 1 Owen Taylor 2001-09-19 20:51:05 UTC
You can do gtk_widget_set_direction() on the entry, to get
right aligned text in LTR locales. However, the effect will
not be quite what you want ...

 123!

entered will display as !123. So it's not really a solution.
API additions will need to wait for GTK+-2.2, however.
Comment 2 Owen Taylor 2002-10-16 19:06:01 UTC
*** Bug 68475 has been marked as a duplicate of this bug. ***
Comment 3 ean 2002-12-07 09:54:08 UTC
Created attachment 12815 [details] [review]
Enables adjustment of text in GtkEntry (right, left and center)
Comment 4 Owen Taylor 2002-12-07 18:19:52 UTC
Hmm, thinking about a bit more, I think this fits into the 
category of alignment, rather than justification.

void    gtk_misc_set_alignment (GtkMisc *misc,
                                gfloat   xalign,
                                gfloat   yalign);
oid       gtk_frame_set_label_align  (GtkFrame      *frame,
                                       gfloat         xalign,
                                       gfloat         yalign);
void                    gtk_tree_view_column_set_alignment      
(GtkTreeViewColumn       *tree_column,
                                                                  gfloat
           xalign);

Justification in GTK+ generally refers to the alignment of multiple
lines within a paragraph.

As for the interaction with gtk_text_direction(), when
gtk_widget_get_direction(widget) == GTK_TEXT_DIR_RTL, 
it should use alignment = 1 - entry->alignment.

So, a float, with 0.0 == left (or for RTL, right) aligned
, 1.0 == right (resp., left) aligned.

Comment 5 ean 2002-12-08 11:04:20 UTC
Created attachment 12837 [details] [review]
Enables any alignment of text in GtkEntry
Comment 6 Owen Taylor 2003-03-28 20:29:43 UTC
Review comments:

 - A temporary variable - 

    text_width = logical_rect.width / PANGO_SCALE;

   would make the computation clearer. (Actually, better
   PANGO_PIXELS (logical_rect.width))

 - The math looks right, but it took me 5-10 minutes
   to verify that. I don't think the MIN/MAX approach
   scales for clarity. I would suggest (untested)

   if (gtk_widget_get_direction (GTK_WIDGET (entry)) == GTK_TEXT_DIR_LTR)
      alignment = entry->alignment
   else
      alignment = 1.0 - entry->alignment;

   text_width = PANGO_PIXELS (logical_rect.width);

   if (text_width > text_area_width)
     {
        min_offset = 0;
        max_offset = text_width - text_area_width;
     }
   else
     {
       min_offset = - (text_area_width - text_width) * alignment;
       max_offset = min_offset;
     }

Which more or less obviously correct.

 - Need a property (and notification on the property
   when calling the setter)

 - Needs real documentation comments 
   "Sets the alignment for the entry." Doc comment should
   say something that isn't just a repetition of the 
   function name, or why have doc comments?
 
   What does the alignment mean? What are the possible
   values?

 - Need the appropriate g_return_[val_]if_fail guards.

 - Needs coding style fixage - see pango/docs/TEXT/coding-style
   for a overview that basically is the same as the
   GTK+ coding style. (Main difference is that GLib/GTK+
   use gint not int)
Comment 7 ean 2003-03-29 08:16:35 UTC
I think your handling of text direction RTL is wrong.
It is scrooling text of the wrong end.
Comment 8 Owen Taylor 2003-03-29 14:03:39 UTC
I haven't tested it, so if it is actually behaving wrong
on testing, well, then it's wrong. But if you look 
at the expression in gtkentry.c currently, you'll see
that when text_width > text_area_width, you get 

 min_offset = 0
 max_offset = text_width - text_area_width

for both LTR and RTL. And this makes sense - a negative
offset corresponds to white space at the left end of
the entry, which you shouldn't get if there is scrolling,
no matter whether you are in LTR or RTL mode.
Comment 9 ean 2003-03-29 17:14:20 UTC
I think my brain was still thinking of the else-branch as being the
RTL-case.

Perhaps we could make it even clearer by removing the min_offset and
max_offset and set the entry->scroll_offset directly.
I can't really see that the CLAMP is doing anything good.

But just as in my original change which only changed two lines, I'm in
general changing as little as possible in the code, as there might be
things as the original code-writer has thought about that I may mis if
I re-write the code.

My re-written proposal would then be:

  text_width = PANGO_PIXELS (logical_rect.width);

  if (gtk_widget_get_direction (GTK_WIDGET (entry)) == GTK_TEXT_DIR_LTR)
     {
     /* Text direction: Left to right */
     if (text_width <= text_area_width)
        {
        entry->scroll_offset = (text_width - text_area_width) *
entry->alignment;
        }
     else
        {
        entry->scroll_offset = text_width - text_area_width;
        }
     }
  else
     {
     /* Text direction: Right to left */
     if (text_width <= text_area_width)
        {
        entry->scroll_offset = (text_width - text_area_width) * (1.0 -
entry->alignment);
        }
     else
        {
        entry->scroll_offset = 0;
        }
     }
Comment 10 Owen Taylor 2003-03-30 00:36:53 UTC
The clamp, is I'm pretty sure necessary.

I suspect that with your patch, the effect will be that
the entry is "spring-loaded" - it won't scroll when
the cursor gets to one end of the entry or the other, 
rather it will scroll back immediately when the user
isn't at the end of the entry.

The basic idea of the min_offset and max_offset is
that we should only change the scroll_offset when
we _have_ to change the scroll offset.
Comment 11 ean 2003-03-30 08:17:15 UTC
I think this just shows the old knowledge - don't change things that
are working :-)

I did of course forget that the cursor do not need to be at the end of
the text. Back in november/december this was all clear to me, but
there has been so much going on since.

Hopefully someone can complete the implementation, as I know I won't
get time for this in the near future :-(
Comment 12 Owen Taylor 2004-02-24 14:13:42 UTC
*** Bug 135177 has been marked as a duplicate of this bug. ***
Comment 13 Steffen Gutmann 2004-02-26 02:58:39 UTC
I compared the solutions of Egon's patch (59799) with the solutuon I
wrote (135177) and Egon's solution seems more compact and elegant. 
While I was computing a position of the layout depending on the
alignment/justification, he simply changed the scroll_offset.  This
needs less changes and looks more elegant than mine.

I therefore vote for giving up my solution (135177) in favor for
Egon's (59799).  I am willing to updated his changes according to the
code review posted by Owen.

Before I am starting doing this I once want to discuss again with Owen
and others the topic of justification (enum of left, right, center)
versus alignment (a float in [0;1]).

I strongly vote for using justification instead of alignment because
of the following reasons:

o Alignment as described in GtkMisc does 'enable the widget to be
positioned within its allocated area. Note that if the widget is added
to a container in such a way that it expands automatically to fill its
allocated area, the alignment settings will not alter the widgets
position.'  This means that the whole GtkEntry (including borders?)
should be aligned, not just the text inside the GtkEntry.

o Alignment means x and y alignment, but we only provide x alignment.
 And I do not see any sense for a y alignment, the height of the
GtkEntry should always be such that only one line of text fits in there.  

o People rather think of justification than of alignment.  The first
version of the patch here (59799) as well as in the one I started
(135177) used justification and not alignment.  This indicates that
people accept justification rather than alignment.

o Pango also only has left, right and center justification (but uses
the term alignment for this).

o I don't see any sense in having a GtkEntry aligned with a value of
e.g. 0.1.  My guess it will look just strange.  For a label I can
imaging to have it aligned with a value of 0.1 (so mainly left but
with some extra space if available). But in a text editor, you only
have left, right and center justification.

o The only good point about alignment is that it is more general than
justification, but that should not mean that we should favor it.  Gtk
is strong in its concepts and we should think well about extensions
such that they make really sense.

o alignment needs a gfloat to be added to the struct GtkEntry
increasing the size by 4? bytes, justification only needs 2 bits which
come for free as we can include them in the bitfield already present
(so the size stays same).

Now, I would like to see what others think, and once we made our
conclusion about this topic, I can start doing the patch which
hopefully will make it into the next gtk version.

-- Steffen
Comment 14 Owen Taylor 2004-02-26 15:13:19 UTC
Thanks for the comments, but they don't convince me. Consistency
with GtkLabel is the most important thing here, and we have
enough problems with people trying to use justification for
GtkLabel instead of alignment without having them say
"But for GtkEntry, I used justification!".

Another strong point in favor of alignment is that the RTL flipping
is much more natural with the numeric alignment value.
Comment 15 Christian Reis (not reading bugmail) 2004-02-28 16:01:33 UTC
I have no real problem with alignment being used here. It's certainly
"feels" more consistent when you take into account RTL and LTR text in
the widget.
Comment 16 Steffen Gutmann 2004-03-01 02:32:37 UTC
Created attachment 24958 [details] [review]
patch for gtkentry for having text aligned, includes changes according to Owen's comments
Comment 17 Steffen Gutmann 2004-03-01 02:42:21 UTC
So shall it be alignment then.  I just added a new patch for your
review.  

Some comments:

I moved the alignment field to the end of struct GtkEntry (out of the
public area) and renamed it to xalign.  I called the property 'xalign'
similar to other classes (GtkMisc for example).  However, the methods
for setting and getting the alignment are called
gtk_entry_set_alignment and gtk_entry_get_alignment.  I wonder if they
should be called differently or if they should take two (and return
two) arguments like the other classes do?  There is GtkFrame which
also alows an yalign but ignores the value currently.  Let me know and
I can change it accordingly.

-- Steffen
Comment 18 Owen Taylor 2004-03-01 03:31:44 UTC
Applied with various fixups. (API freeze is tomorrow, so
I'd like to just get it in at this point.)

* moved xalign into instance-private-data. Adding fields to
  the end of the structure breaks binary compatibility.

* Indentation - in function definitions the return
  type goes on a separate line.

* Use P_() for property name/description translation. 
  This allows translators to separate out user visible
  strings.

* Changed the property description to:

   P_("The horizontal alignment, from 0 (left) to 1 (right). Reversed
fo\r RTL layouts.")

  which I think makes things more obvious.

* No need to freeze/thaw the notify queue when only notifying
  on one property.

* Instead of calling gtk_widget_queue_draw(), call
  gtk_entry_recompute(), since simply redrawing won't
  changing the scroll offset.

* Extend the gtk_entry_set_alignment() 

I think the single parameter gtk_entry_set_alignment() works
well and see no reason to change it.

Sun Feb 29 22:01:49 2004  Owen Taylor  <otaylor@redhat.com>
                                                                     
                                                       
        * gtk/gtkentry.[ch]: Add gtk_entry_set_alignment()
        to allow right-aligned entries and a "xalign"
        property. (#59799, patch from Egon Andersen and
        Steffen Gutmann)