Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.0][com_media] RTL: adapting media manager to rtl + some #24043

Merged
merged 6 commits into from
Feb 28, 2019

Conversation

infograf768
Copy link
Member

@infograf768 infograf768 commented Feb 28, 2019

Summary of Changes

This PR corrects the rtl display of the media manager.
Plus:
It also modifies 2 language strings (dates) and allows Jalali date to be displayed.
It adds a missing lang string.
It adds to the frontend com_media.ini the missing lang strings to let create site only lang packs.
It orders language strings in the ini files as well as in default_texts.php

What it does not do:
It does not delete the frontend language strings which are no more used.
It does not solve the frontend infobar absent display.
It does not solve frontend Inserting image in article TypeError: Joomla.getImage is not a function

Testing Instructions

Install the Persian Language pack (administrator/index.php?option=com_installer&view=languages)
Switch to Persian in administrator.
Display the media manager.

Patch and run npm ci

Before patch

This is what we get for LTR

screen shot 2019-02-28 at 08 23 57ltr

This is what we get for RTL

screen shot 2019-02-28 at 08 19 20rtlbefore

After patch

This is what we should get for RTL

screen shot 2019-02-28 at 12 09 33

Supplementary test

After patch and npm ci
Enable Debug lang.
Delete or change name of administrator/language/en-GB/en-GB.com_media.ini in order to load ONLY the front end file.
Edit an article in front end and display the xtd Images modal
There should not be any more untranslated strings

Documentation Changes Required

None

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Feb 28, 2019
COM_MEDIA_UP="Up"
COM_MEDIA_UPLOAD="Upload"
COM_MEDIA_UPLOAD_COMPLETE="Upload Complete"
COM_MEDIA_UPLOAD_FILE="Upload file"
COM_MEDIA_UPLOAD_SUCCESS="Item uploaded."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need the fullstop (.) at the end?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not corrected various existing lang strings.
We do have already
COM_MEDIA_DELETE_SUCCESS="Item deleted." for example.
or
COM_MEDIA_ERROR="An error occurred."
I think such messages should end with a period.

As you can see the capitals and periods for these are not consequent.
I can though modify them on the model of
COM_CATEGORIES_SAVE_SUCCESS="Category saved." etc.

In this case, we should have I guess

COM_MEDIA_UPLOAD_COMPLETE="Upload complete."
COM_MEDIA_UPLOAD_FILE="Upload file"
COM_MEDIA_UPLOAD_SUCCESS="Item uploaded."

I guess these can be modified later on by specialists in a specific PR, to not interfere with this PR.
What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks also that
COM_MEDIA_UPLOAD_FILE is not used at all in 4.0

These ini will have to be drastically cleaned up when media manager gets more stable.

@wilsonge wilsonge merged commit 94ede4e into joomla:4.0-dev Feb 28, 2019
@wilsonge
Copy link
Contributor

Thanks!

@infograf768 infograf768 deleted the 4.0_media_rtl branch February 28, 2019 13:52
@zero-24 zero-24 added this to the Joomla 4.0 milestone Mar 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants