Left-Align Tooltips

Left-Align Tooltips

Aligning tooltips to the left (or to the right according to the Locale
UI layout and the text directionality).

R=sky@chromium.org
BUG=334457
TEST=manual

Committed: https://crrev.com/2c042d92610f967b8b13f4f4bfb0f461983b888a
Cr-Commit-Position: refs/heads/master@{#314171}

[sky, 2015-01-28 21:24:59]

https://codereview.chromium.org/864223004/diff/1/ui/views/corewm/tooltip_aura.cc
File ui/views/corewm/tooltip_aura.cc (right):

https://codereview.chromium.org/864223004/diff/1/ui/views/corewm/tooltip_aura...
ui/views/corewm/tooltip_aura.cc:48: base::i18n::TextDirection dir =
base::i18n::GetStringDirection(text);
I think you should be able to set the alignment to LEFT and have everything just
work. In other words it seems like label so deal with rtl and alignment for
text.

[afakhry, 2015-01-28 21:36:13]

https://codereview.chromium.org/864223004/diff/1/ui/views/corewm/tooltip_aura.cc
File ui/views/corewm/tooltip_aura.cc (right):

https://codereview.chromium.org/864223004/diff/1/ui/views/corewm/tooltip_aura...
ui/views/corewm/tooltip_aura.cc:48: base::i18n::TextDirection dir =
base::i18n::GetStringDirection(text);
On 2015/01/28 21:24:59, sky wrote:
> I think you should be able to set the alignment to LEFT and have everything
just
> work. In other words it seems like label so deal with rtl and alignment for
> text.

That's true, but Label::SetHorizontalAlignment() only takes into account the
RTL/LTR of the UI, but the text directionality is ignored.

So for example, if the UI is set to be RTL,
Label::SetHorizontalAlignment(ALIGN_LEFT) will actually align right for all
texts. If we have a tool tip in English and the UI is in Arabic, the English
tooltip will be aligned right (which is wrong).

Similarly, if we have the UI in English, and a Tool tip in Arabic, Aligning left
will still align the Arabic tool tip left (which is also wrong).

[sky, 2015-01-29 00:58:35]

I agree that is what happens now. My thinking is we likely want exactly the
same logic you're trying to add to tooltip now in label. Is there a reason
not to implement the logic there?

  -Scott

On Wed, Jan 28, 2015 at 1:36 PM, <afakhry@chromium.org> wrote:

>
> https://codereview.chromium.org/864223004/diff/1/ui/views/
> corewm/tooltip_aura.cc
> File ui/views/corewm/tooltip_aura.cc (right):
>
> https://codereview.chromium.org/864223004/diff/1/ui/views/
> corewm/tooltip_aura.cc#newcode48
> ui/views/corewm/tooltip_aura.cc:48: base::i18n::TextDirection dir =
> base::i18n::GetStringDirection(text);
> On 2015/01/28 21:24:59, sky wrote:
>
>> I think you should be able to set the alignment to LEFT and have
>>
> everything just
>
>> work. In other words it seems like label so deal with rtl and
>>
> alignment for
>
>> text.
>>
>
> That's true, but Label::SetHorizontalAlignment() only takes into account
> the RTL/LTR of the UI, but the text directionality is ignored.
>
> So for example, if the UI is set to be RTL,
> Label::SetHorizontalAlignment(ALIGN_LEFT) will actually align right for
> all texts. If we have a tool tip in English and the UI is in Arabic, the
> English tooltip will be aligned right (which is wrong).
>
> Similarly, if we have the UI in English, and a Tool tip in Arabic,
> Aligning left will still align the Arabic tool tip left (which is also
> wrong).
>
> https://codereview.chromium.org/864223004/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[afakhry, 2015-01-29 01:22:04]

On 2015/01/29 00:58:35, sky wrote:
> I agree that is what happens now. My thinking is we likely want exactly the
> same logic you're trying to add to tooltip now in label. Is there a reason
> not to implement the logic there?
> 
>   -Scott
> 
> On Wed, Jan 28, 2015 at 1:36 PM, <mailto:afakhry@chromium.org> wrote:
> 
> >
> > https://codereview.chromium.org/864223004/diff/1/ui/views/
> > corewm/tooltip_aura.cc
> > File ui/views/corewm/tooltip_aura.cc (right):
> >
> > https://codereview.chromium.org/864223004/diff/1/ui/views/
> > corewm/tooltip_aura.cc#newcode48
> > ui/views/corewm/tooltip_aura.cc:48: base::i18n::TextDirection dir =
> > base::i18n::GetStringDirection(text);
> > On 2015/01/28 21:24:59, sky wrote:
> >
> >> I think you should be able to set the alignment to LEFT and have
> >>
> > everything just
> >
> >> work. In other words it seems like label so deal with rtl and
> >>
> > alignment for
> >
> >> text.
> >>
> >
> > That's true, but Label::SetHorizontalAlignment() only takes into account
> > the RTL/LTR of the UI, but the text directionality is ignored.
> >
> > So for example, if the UI is set to be RTL,
> > Label::SetHorizontalAlignment(ALIGN_LEFT) will actually align right for
> > all texts. If we have a tool tip in English and the UI is in Arabic, the
> > English tooltip will be aligned right (which is wrong).
> >
> > Similarly, if we have the UI in English, and a Tool tip in Arabic,
> > Aligning left will still align the Arabic tool tip left (which is also
> > wrong).
> >
> > https://codereview.chromium.org/864223004/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

You're right, we probably want that in Label. But I'm not sure about the
assumptions made by the current users of Label.

[afakhry, 2015-01-31 01:44:11]

Apparently there was no need for all that complexity, all we needed to do was to
use: gfx::ALIGN_TO_HEAD;

[sky, 2015-02-02 15:44:34]

Nice. I like simple:)

LGTM

[afakhry, 2015-02-02 17:18:40]

The CQ bit was checked by afakhry@chromium.org

[commit-bot: I haz the power, 2015-02-02 17:19:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/864223004/20001

[commit-bot: I haz the power, 2015-02-02 18:41:35]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-02-02 18:42:35]

Patchset 2 (id:??) landed as
https://crrev.com/2c042d92610f967b8b13f4f4bfb0f461983b888a
Cr-Commit-Position: refs/heads/master@{#314171}