744982 - Decide whether GCLI in the global toolbar should be fixed direction:ltr or not

Status: RESOLVED FIXED

(DevTools :: Console, defect)

Description

[Joe Walker [:jwalker]]

The command line in the web console is fixed left-to-right. It was felt that even in rtl languages, using ltr was more natural.

Like the web console, GCLI's input is not localized, and shouldn't be, however the output is.

I've played with GCLI in rtl, mode and it's not 100% clear to me which parts should be fixed ltr.

When the developer toolbar (which embedds GCLI) has landed (bug 720641) we should get input from people who natively understand rtl.

Comment 1

[Dão Gottwald [:dao]]

I'm not a fan of the "land and then see whether we need to fix it" approach. Can you point Ehsan to a try build so we can get this cleared up as part of the review for bug 720641?

Comment 2

[Joe Walker [:jwalker]]

I'm not shirking off work here. We're doing the same as the web console, so it's probably OK, but I want to make sure.

Also, it's easier to get Ehsan's help when I can point him at a preference rather than at a try build.

Comment 3

[Joe Walker [:jwalker]]

That might sound like I'm against Ehsan using a try build - I'm not. I just don't want to get held up.

Ehsan, the latest is here: https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-528413994a34/

Dão - I've also created a meta bug of the things we need to fix before we can prev this on - Bug 745773, this bug blocks that.

Comment 4

[(no longer active)]

So I took a look at the try server build (thanks Joe for making it for me!).  Can you please tell me what GCLI is, and how I can test it?  I took a look under the Web Developer menu in this build, but did not find anything unfamiliar!

Also, feel free to ping me on IRC if you want really fast replies (although I'll try to respond quickly.)

Comment 5

[Joe Walker [:jwalker]]

Thanks Ehsan. You need to set a pref: devtools.toolbar.enabled = true, then you should get an extra 'Developer Toolbar' option in the Developer Tools menu.

There is a bug to review the UI (bug 744906), so don't worry that it doesn't look perfect yet.

There are 3 areas to think of
- the command output area (what you see when you type a command e.g. 'help',
  on the right hand side of the command-line in ltr mode)
- the command hint area (what you see when you type an wrong command or press F1,
  on the left hand side of the command-line in ltr mode)
- the toolbar with embedded command-line

Currently, like the web console, all 3 areas are fixed ltr. But I expect that we'll want to change the output area and the hint area to not be fixed. The text put into these areas is localized - it's not 'code'. (Also we'll likely be merging them into one as part of the UI update)

I don't expect that the toolbar itself would be fixed ltr, but the commands inside the toolbar are not localized - so perhaps that should be fixed ltr?

Then would it be weird if the toolbar was presented in rtl mode with the buttons on the left, and the command line on the right, but with the prompt on the left?

I hope that makes sense.

Comment 6

[(no longer active)]

(In reply to Joe Walker from comment #5)
> There are 3 areas to think of
> - the command output area (what you see when you type a command e.g. 'help',
>   on the right hand side of the command-line in ltr mode)

It's fine if this appears LTR for RTL builds, but we do want to make the description texts right-aligned and RTL, like this:

  break       !TXET LTR EMOS
  tilt   .TXET LTR EROM EMOS

(upper-case chars representing RTL chars).

> - the command hint area (what you see when you type an wrong command or
> press F1,

This should definitely be LTR.  But the text for the help popups should be right-aligned and RTL, and they should snap on the left side of the command text box to make them visually related to the typed in command.

>   on the left hand side of the command-line in ltr mode)
> - the toolbar with embedded command-line

The toolbar itself should be RTL, but the textbox inside it should be LTR and the text should be left aligned, like:

 +--------+ +--------+  +-------------------------------------------+
 | button | | button |  | <command text box>                        |
 +--------+ +--------+  +-------------------------------------------+

Hope this helps.

BTW, when testing this I hit a pretty bad bug.  If you click "REPL" in the popup which is initially displayed, the wikipedia page gets loaded *inside* the popup itself, as opposed to in a new background tab.  And this will cause the future popups to have the wikipedia page in the background.

Also, note that the wikipedia link is broken.

Comment 7

[Joe Walker [:jwalker]]

(In reply to Ehsan Akhgari [:ehsan] from comment #6)
> (In reply to Joe Walker from comment #5)
> > There are 3 areas to think of
> > - the command output area (what you see when you type a command e.g. 'help',
> >   on the right hand side of the command-line in ltr mode)
> 
> It's fine if this appears LTR for RTL builds, but we do want to make the
> description texts right-aligned and RTL, like this:
> 
>   break       !TXET LTR EMOS
>   tilt   .TXET LTR EROM EMOS
> 
> (upper-case chars representing RTL chars).
> 
> > - the command hint area (what you see when you type an wrong command or
> > press F1,
> 
> This should definitely be LTR.  But the text for the help popups should be
> right-aligned and RTL, and they should snap on the left side of the command
> text box to make them visually related to the typed in command.
> 
> >   on the left hand side of the command-line in ltr mode)
> > - the toolbar with embedded command-line
> 
> The toolbar itself should be RTL, but the textbox inside it should be LTR
> and the text should be left aligned, like:
> 
>  +--------+ +--------+  +-------------------------------------------+
>  | button | | button |  | <command text box>                        |
>  +--------+ +--------+  +-------------------------------------------+
> 
> Hope this helps.

It does, thanks. I'll make sure that as we land the UI update we get all this, and I'll come back again for you to check if that's OK.


> BTW, when testing this I hit a pretty bad bug.  If you click "REPL" in the
> popup which is initially displayed, the wikipedia page gets loaded *inside*
> the popup itself, as opposed to in a new background tab.  And this will
> cause the future popups to have the wikipedia page in the background.
> 
> Also, note that the wikipedia link is broken.

Outch - thanks. I raised bug 746499.

Comment 8

[Joe Walker [:jwalker]]

GCLI Triage.

Comment 9

[Joe Walker [:jwalker]]

Attachment: Upload 1

Ehsan - Both output panels are supposed to be translated in general - command output included, so I've made this switchable direction. In fact my understanding is that now the only thing that's fixed ltr is actual commands.

2 things need thought:
- The way I've made the HTML documents follow the direction of the top level
  window feels slightly hacky, but I've not found anything less hacky that
  actually works.
- I may have gone too far with the having everything switchable direction and
  I'm wondering if the command suggestion table (what pops up when you press F1
  in an empty command line) should be fixed ltr. Perhaps we should file a
  followup for that.

Comment 10

[(no longer active)]

Is there a try server build I can use to test?  If not I can build this myself...

Comment 11

[Joe Walker [:jwalker]]

(In reply to Ehsan Akhgari [:ehsan] from comment #10)
> Is there a try server build I can use to test?  If not I can build this
> myself...

I'll get you one.

Comment 12

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 627193 [details] [diff] [review]
Upload 1

I like 1.6KB patches.

Presumably the toolbar's styles.direction will be set by the system and your dir attribute will be correct.

_div is the output area, right?

and then again on the hintElement.

You're repeating that getComputedStyle call twice. Maybe throw that in a getter somewhere?

+#gcli-output-root[dir=rtl],
+#gcli-tooltip-root[dir=rtl] {
+  direction: rtl
+}
+

That's in winstripe only. Do we need this in other themes?

I'm inclined to believe this was a left-over from before you switched to using the dir attribute.

Comment 13

[Joe Walker [:jwalker]]

(In reply to Rob Campbell [:rc] (:robcee) from comment #12)
> Comment on attachment 627193 [details] [diff] [review]
> Upload 1
> 
> I like 1.6KB patches.
> 
> Presumably the toolbar's styles.direction will be set by the system and your
> dir attribute will be correct.

Right.

> _div is the output area, right?
> 
> and then again on the hintElement.

Correct.

> You're repeating that getComputedStyle call twice. Maybe throw that in a
> getter somewhere?

They're on different classes (OutputPanel|ToolbarPanel) so it's not easy. I think we'd make things far more verbose by adding that in - ok?

> +#gcli-output-root[dir=rtl],
> +#gcli-tooltip-root[dir=rtl] {
> +  direction: rtl
> +}
> +
> 
> That's in winstripe only. Do we need this in other themes?
> 
> I'm inclined to believe this was a left-over from before you switched to
> using the dir attribute.

Ignore everything I said on IRC. It's not needed.
Thanks.

Comment 14

[Joe Walker [:jwalker]]

Attachment: Upload 2

Comment 15

[Joe Walker [:jwalker]]

https://tbpl.mozilla.org/?tree=Try&rev=1874a51a06a7

Comment 16

[Joe Walker [:jwalker]]

Ehsan, promised try build:
https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-1874a51a06a7/

Comment 17

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 627937 [details] [diff] [review]
Upload 2

I like 1.24KB patches EVEN better!

Comment 18

[(no longer active)]

Comment on attachment 627937 [details] [diff] [review]
Upload 2

The direction of the arrows in the help panel needs to be flipped as well.  Looks very good otherwise.

Comment 19

[Rob Campbell [:rc] (:robcee)]

(In reply to Joe Walker from comment #13)
> (In reply to Rob Campbell [:rc] (:robcee) from comment #12)
> > Comment on attachment 627193 [details] [diff] [review]
> > You're repeating that getComputedStyle call twice. Maybe throw that in a
> > getter somewhere?
> 
> They're on different classes (OutputPanel|ToolbarPanel) so it's not easy. I
> think we'd make things far more verbose by adding that in - ok?

sure thing.

Comment 20

[Joe Walker [:jwalker]]

Attachment: Upload 3

Comment 21

[Joe Walker [:jwalker]]

https://tbpl.mozilla.org/?tree=Fx-Team&rev=a24414165cd4

Comment 22

[Rob Campbell [:rc] (:robcee)]

https://hg.mozilla.org/mozilla-central/rev/66f127caa6fa