cockpit
cockpit copied to clipboard
shell: Add basic language RTL support
Addresses #14058
Much more work is needed to fix the UI everywhere, but this is a start.
Ideally, we'd want to have the RTL check in a function that both places used, but I wanted to get a very simple proof of concept started.
There are a bunch of issues all over Cockpit, mainly float-related (we shouldn't be using floats for layouts anymore anyway) and margins (one of the many reasons why it's better to use grid/flex and gap).
Additionally, some layouts are flipped that shouldn't be, but, until now, we didn't have a way to adjust for them in the CSS. (Nor did we have a reason to, without any RTL support, including RTL languages.)
For CSS, we can branch layout decisions with
[dir=ltr] .foo {
margin-right: 1rem;
}
[dir=rtl] .foo {
margin-left: 1rem;
}
Inside of SCSS, we can do things like:
.foo {
[dir=ltr] & {
margin-right: 1rem;
}
[dir=rtl] & {
margin-left: 1rem;
}
}
Which could help with keeping blocks together.
Not everything would need both LTR and RTL — many things "just work". And sometimes, we can get away with just having the RTL support to override the LTR one.
Additionally, our supported browsers all support unset too, so we could do:
.foo {
margin-right: 1rem;
[dir=rtl] & {
margin-right: unset;
margin-left: 1rem;
}
}
(Or just set the margin
, either via shorthand or by giving it a value... or consider flex or grid.)
Obviously, there's a lot more than setting margins too. :wink: This is just one very probable example.
We may want to also get rid of float: left
and (especially) float: right
, or at least evaluate their usage. (They also can easily break accessibility and are at least somewhat fragile.)
@yarons: I hope this is a good start. Are you able to check out this branch and compile Cockpit locally to check things out? We have a guide @ https://cockpit-project.org/external/source/HACKING
I changed var
to let
in the two places lang_dir
is initialized. (I didn't change any of the other variables that are set in the same code blocks.)
As I assumed and is stated above, this is probably not the proper place within Cockpit to change the language direction. It's just a starter so we can see RTL in action. (And hopefully others will help fix this more properly.) Basically: this PR is using The Stone Soup model. :grin:
I've added some login support as requested. It's also quite basic. I've used var instead of let, as the login page needs to support old browsers, such as Internet Explorer (minimally to tell them they're not supported). Throwing a JavaScript error would prevent that.
The JS modifications I've made work enough, but should really be replaced by someone else. :wink:
Hey guys, sorry I'm not on it for the past couple of days but I'm still struggling to create a working dev environment using docker on mac, sadly this is the only computer I have available right now.
@yarons: I was talking with @andreasn about this PR last week and suggested we set up a VM for testing, with this branch compiled in. Hopefully we can get something together soon for you (and others) to test, to start filing issues.
@garrett this is awesome, I've opened a bug (#14128) regarding the Vagrant problem but nothing happened yet.
@yarons: right, sorry -- none of us uses vagrant, we have more convenient and efficient ways to do development. (Personally I never got vagrant to work properly either, with cockpit or otherwise, so I never bothered)
Nah, that's fine, VM is much better, what is the deployment policy? Is there a restricted branch with autopulling?
@yarons: We don't do any auto-deployment, as Cockpit isn't a real web service. This is how to build a VM from a branch:
export TEST_OS=fedora-32
git clone -b language-rtl https://github.com/garrett/cockpit.git
cd cockpit
tools/make-bots
test/image-prepare $TEST_OS
bots/vm-run -s cockpit.socket $TEST_OS
and then point your browser to http://127.0.0.2:9091 .
You need the build and test dependencies installed for that.
For $TEST_OS
you can also use "debian-stable" or "ubuntu-2004" or "centos-8-stream" depending on your preferences, but it pretty well doesn't matter for this purpose.
yaron@machine:~/workspace/cockpit$ ./tools/make-bots
info: Could not add alternate for '/home/yaron/.cache/cockpit-project/bots': path '/home/yaron/.cache/cockpit-project/bots' does not exist
checked out bots/ ref d5f63f539aad2ba0a9622223b43dc06cf7cf259f
yaron@machine:~/workspace/cockpit$ ./test/image-prepare $TEST_OS
Traceback (most recent call last):
File "./test/image-prepare", line 212, in <module>
sys.exit(main())
File "./test/image-prepare", line 74, in main
results = os.path.join(testvm.get_temp_dir(), "build-results")
AttributeError: module 'testvm' has no attribute 'get_temp_dir'
Well, that's not good either :(
@yarons : Ah, this branch is rather stale -- it needs to be rebased to current master.
Rebased to master.
@yarons: I hope this compiles better for you now!
Rebased to current code & force-pushed
Rebased to master.
- This still needs someone (@martinpitt, @marusak, etc.) to pick it up and polish the code to be better than my little hack. :wink:
- And then we'll need assistance from a native speaker (and reader) of Hebrew to help us find and fix the issues. (This can already be done with my hacky implementation.)
- And I'll probably need to go in and fix various CSS related issues too.
To make it clear on how to use this (as I think it might not be clear enough from the above discussion):
- compile this branch based on the instructions at https://cockpit-project.org/external/source/HACKING
- visit Cockpit in your browser (probably at https://localhost:9090/)
- sign in
- switch the display language to Hebrew
The dialog looks like this when you're switching:
(It would be ideal if we could have a test VM already compiled and set up somewhere for anyone who is a native Hebrew speaker and wants to volunteer to help us fix up RTL, so they wouldn't have to compile Cockpit.)
I have an Ubuntu VM running Cockpit from the repos, what can I do to use the relevant revision with Hebrew?
@yarons: I've compiled Cockpit for Ubuntu 20.04 and have included the (non-debug) debs in a zip (to upload here on GitHub) for you to test in your VM:
🡇 ubuntu-20.04-24a587ffb.zip (23.8 MB)
Hopefully this helps! Please let us know here.
I've rebased and rebuild the packages, for both Ubuntu and Fedora:
🡇 cockpit-ubuntu-20.04-gfd67bebaf.zip (18.7 MB) 🡇 cockpit-fedora-33-gfd67bebaf.zip (20.9 MB)
These packages are this branch, rebased on the current code we have in git as of today.
OK, there are some glitches.
![Screen Shot 2020-11-15 at 1 11 06](https://user-images.githubusercontent.com/406826/99158810-a273e980-26df-11eb-9436-35fa7b4a6420.png)
![Screen Shot 2020-11-15 at 1 10 01](https://user-images.githubusercontent.com/406826/99158823-c59e9900-26df-11eb-94cd-26317b26d299.png)
![Screen Shot 2020-11-15 at 1 09 52](https://user-images.githubusercontent.com/406826/99158837-e961df00-26df-11eb-8152-9d2a724b62c7.png)
![Screen Shot 2020-11-15 at 1 09 41](https://user-images.githubusercontent.com/406826/99158856-10201580-26e0-11eb-9426-d54abdb53a25.png)
![Screen Shot 2020-11-15 at 1 09 23](https://user-images.githubusercontent.com/406826/99158876-4cec0c80-26e0-11eb-9af8-0a084d76722f.png)
![Screen Shot 2020-11-15 at 1 09 08](https://user-images.githubusercontent.com/406826/99158889-69884480-26e0-11eb-8bbc-398d9506d76c.png)
![Screen Shot 2020-11-15 at 1 08 19](https://user-images.githubusercontent.com/406826/99158919-a8b69580-26e0-11eb-938a-922508305f75.png)
![Screen Shot 2020-11-15 at 1 08 05](https://user-images.githubusercontent.com/406826/99158938-c1bf4680-26e0-11eb-87ae-51227c8311bc.png)
![Screen Shot 2020-11-15 at 1 07 53](https://user-images.githubusercontent.com/406826/99158955-e74c5000-26e0-11eb-9ff9-f3ee8291cba9.png)
Rebased.
However, PF4 doesn't support RTL yet, so it might be difficult for us to do so properly.
Rebased, just to keep this up-to-date.
Rebased again, just to keep the branch from going stale.
Rebased so this doesn't get too stale.
Rebased again, to prevent staleness. Thankfully it still cleanly applies, so bringing it forward is easy.
Rebased.
Rebased again.
We still need:
- The team to dedicate time for this.
- As a team, fix the RTL layout problems. (It cannot just be me.)
- Someone on the team to more properly implement the switching (and it shouldn't be just for Hebrew, although that's still OK for a first pass).
- Someone to help with this who knows Hebrew and can spare the time during the process who can build this PR and test it on their machine. (This should be during the time that we work on this.)
- Probably some PF fixes, where they're using margin instead of flex or grid (which is still a problem in PF for some odd reason, which also affects some layouts in LTR and mobile mode too). Even with some rough spots of RTL, we can generally fix a ton and make it mostly work, especially for a first official pass.
I'll volunteer for the Hebrew testing, where can I find instructions for this? I'll prepare a VM for that.
So Intl is not supported on firefox, so that is no go unfortunately. I though we could declare this in manifest or in cockpit
but we don't have neither of these on login page. So I think using cookie will be the best approach as we already use it for the language. I set it up when the language is changed, so one corner case is that if someone is already using hebrew then the new version won't switch the order, they will have to force change the language. But honestly, doubt anyone is using it since it is wrong way round.
There are a lot of broken pieces. I would say, let's fix some obvious easy fixes and let's merge this and then do rest in followups.
@marusak there's an open bug but it's pending for several other bugs: https://bugzilla.mozilla.org/show_bug.cgi?id=1693576