search

joomla / joomla-cms

Home of the Joomla! Content Management System
https://www.joomla.org
GNU General Public License v2.0
4.76k stars 3.64k forks source link

CodeMirror always to be left to right direction #42654

Closed sinahaghparast closed 9 months ago

sinahaghparast commented 9 months ago

What needs to be fixed

When language of page is RTL, CodeMirror is RTL... NOT GOOD...

Why this should be fixed

CodeMirror MUST always Left to Right... Because every computer languages script is Left to Right... OR adding parameter in <field name="xxxx" type="editor" label="COM_xxxx_LABEL" editor="codemirror" ... dir="ltr/rtl" <-------------------- Add this attribute ------ />

How would you fix it

when admin side set RTL, and edit template files, CodeMirror NOT Good...

Side Effects expected

brianteeman commented 9 months ago

Other than the line numbers being on the opposite side where is codemirror working in RTL. Seems to work perfectly for me. Please provide more information.

image

richard67 commented 9 months ago

@brianteeman If you check your screenshot again you will see (like I saw now when checking on my 5.1-dev with Farsi language) that the semicolons of the code lines are at the left hand side, and the dollars at the right hand side, so that is a bit RTL-ish, but the rest of the code is LTR, so we have a mix and the code is messed.

P.S.: Doc block comments are also messed, they end with /* instead of */. Maybe the file will be right when it is saved, I haven't checked yet, but editing is a pain.

And e.g. the line ;()app = Factory::getApplication$ ... weird.

Do you think that's a code mirror issue? Or should we fix that in the CMS?

richard67 commented 9 months ago

P.S.: Adding a dir="ltr" attribute to the div with the content (class="cm-content") e.g. with the browser's developer tools solves that issue and the code looks as it should.

brianteeman commented 9 months ago

my bad.

pr incoming. the css in the admin-template-css is incorrect

richard67 commented 9 months ago

@brianteeman If we have that issue also in 4.4-dev we should fix it there, as it is a bug fix and will later be merged up.

richard67 commented 9 months ago

Closing as having a pull request. Please test #42655 . Thanks in advance.

@sinahaghparast I don't know how familiar you are with testing Joomla pull requests, especially such where it is a bit complicated because it touches css or js so it needs to rebuild the media files. So I will post you a few useful links and info now.

You can test such PRs in 2 ways:

Either you have a complete development environment for Joomla, i.e. git clone, composer, npm and so on on a local webserver with a local database. You can find here some information on how to set up such an environment: https://docs.joomla.org/J4.x:Setting_Up_Your_Local_Environment . It might be a bit outdated because once made for 4.0.0, but I think you will get the idea when you are familiar with web development.

Or you use the installation package created by our CI tool "Drone" and make a Joomla installation with that on a testing or development site, i.e. of course not a live site.

There is this document which describes how to test Joomla patches: https://docs.joomla.org/Testing_Joomla!_patches . It might be a bit outdated at the top, but there is a section 10 which describes how to get and use these packages created by Drone for pull requests.

It would really be helpful if you could test Brian's pull request because every non-trivial pull request needs 2 successful human tests to be accepted.

When you have tested the pull request, please comment in the pull request and report your result.

Thanks in advance, and thanks for reporting the issue.

richard67 commented 9 months ago

P.S.: The pull request needs to be tested on 4.4.1 or 4.4.2 or a current 4.4-dev branch.

Update: I can't reproduce the issue on 4.4-dev, see my comment in the PR: https://github.com/joomla/joomla-cms/pull/42655#issuecomment-1890931466 . So it might need to rebase that PR or replace it by a new one.

richard67 commented 9 months ago

New pull request is #42656 . Please test. Linkt to hopefully helpful documents about testing can be found in my previous comment: https://github.com/joomla/joomla-cms/issues/42654#issuecomment-1890927754 . Thanks in advance.

richard67 commented 9 months ago

P.S.: The new pull request is for 5.0-dev, so it should be tested 5.0.x.