Skip to content

Conversation

@khu5h1
Copy link
Contributor

@khu5h1 khu5h1 commented Nov 7, 2021

Pull Request for Issue #18833 (part).

Summary of Changes

Removed Complete & from the open site and open admin buttons in all languages.

Testing Instructions

Install Joomla and after its successful installation, the Congratulations page will have Open site and Open admin buttons.

Actual result BEFORE applying this Pull Request

Complete & appeared on the installation completion page.

Expected result AFTER applying this Pull Request

Complete & is removed from the installation completion page.

Documentation Changes Required

No

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-4.0-dev labels Nov 7, 2021
@richard67
Copy link
Member

It needs to adapt system tests to the changed button texts, see the log here: https://ci.joomla.org/joomla/joomla-cms/48299/1/22

@richard67
Copy link
Member

richard67 commented Nov 7, 2021

Question: Do you really speak all these languages for which you have changed the text? Or have you just removed the first 2 words everywhere without understanding what they mean?

I would expect that this PR changes only the English (GB) texts and leaves the translations to the translation teams.

@khu5h1
Copy link
Contributor Author

khu5h1 commented Nov 7, 2021

Hey @richard67, Thank you for your feedback. I am trying to figure out a way to solve the failing test. Apart from this, I have used Google Translator and have translated all the languages. Then, accordingly, I have committed the changes.

@khu5h1
Copy link
Contributor Author

khu5h1 commented Nov 7, 2021

By the way, May you please help me with some code pointers/references to solve the failing tests.

@richard67
Copy link
Member

@khu5h1 Am not familiar with these tests either but will try to find someone who is.

@khu5h1
Copy link
Contributor Author

khu5h1 commented Nov 7, 2021

Sure @richard67 , I will try to figure out the solution to the failing tests.

@Kubik-Rubik
Copy link
Member

@khu5h1 Thanks for the PR! Please do not modify language files of languages you are not speaking fluently (best practice: only update the main language file en-GB). We have dedicated translation teams with native speakers who will take care of it!

Your change will let this test to fail:

grafik

You need to update this test as well to pass the check.

@dgrammatiko
Copy link
Contributor

The changes should be reflected here: https://github.com/joomla-projects/joomla-browser/blob/95e099d99044ba9915d0d99b31028b723e2fc7e4/src/JoomlaBrowser.php#L306

@wojsmol
Copy link
Contributor

wojsmol commented Nov 7, 2021

@khu5h1 As a member of Polish translation team I support many request above to revert changes to languages other then en-GB.

@wojsmol
Copy link
Contributor

wojsmol commented Nov 8, 2021

@khu5h1 If you revert changes to languages other then English then I will make PR to fix tests.

@Quy Quy added the Updates Requested Indicates that this pull request needs an update from the author and should not be tested. label Nov 8, 2021
@Quy Quy removed the Updates Requested Indicates that this pull request needs an update from the author and should not be tested. label Nov 8, 2021
wojsmol added a commit to wojsmol/joomla-browser that referenced this pull request Nov 8, 2021
@wojsmol
Copy link
Contributor

wojsmol commented Nov 8, 2021

Related PR created joomla-projects/joomla-browser#223

@wojsmol
Copy link
Contributor

wojsmol commented Nov 8, 2021

I have tested this item ✅ successfully on 213ac48

on code review


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35988.

@manav014
Copy link

manav014 commented Nov 8, 2021

I have tested this item ✅ successfully on 213ac48


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35988.

@richard67 richard67 added the Updates Requested Indicates that this pull request needs an update from the author and should not be tested. label Nov 8, 2021
@richard67
Copy link
Member

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35988.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Nov 8, 2021
@richard67 richard67 added RTC This Pull Request is Ready To Commit and removed RTC This Pull Request is Ready To Commit Updates Requested Indicates that this pull request needs an update from the author and should not be tested. labels Nov 8, 2021
@richard67
Copy link
Member

Unfortunately this PR solves the mentioned issue only partly. It removes the "Complete &" but still leaves 2 buttons. So it seems we have to leave that issue open.

@khu5h1
Copy link
Contributor Author

khu5h1 commented Nov 8, 2021

Yeah, we shall keep that issue open and I will create a separate PR for the remaining changes.

@richard67
Copy link
Member

@khu5h1 Then it would be good if you could change the text at the top of your PR's description to show that it solves the issue only party, e.g. by changing from "Pull Request for Issue #18833 ." to "Pull Request for Issue #18833 (part).". Thanks in advance.

@brianteeman
Copy link
Contributor

#18833 (comment)

@richard67
Copy link
Member

#18833 (comment)

@khu5h1 Could you check this comment in the issue? I think it is right, the naming we use is "Administrator" and not "Admin".

@wojsmol If that has been done, could you adjust your PR for the tests?

Thanks in advance both.

I know, it was wrong all the time before, but it would make sense to fix it here and not need another PR for that which again will have to change the tests in the other repository, too.

@khu5h1 khu5h1 changed the title Removed Complete & from open site and open admin button Removed Complete & from open site and open admin button(solves 18833 partially) Nov 8, 2021
@richard67 richard67 changed the title Removed Complete & from open site and open admin button(solves 18833 partially) Removed Complete & from open site and open admin button Nov 8, 2021
@richard67 richard67 added Updates Requested Indicates that this pull request needs an update from the author and should not be tested. and removed RTC This Pull Request is Ready To Commit labels Nov 8, 2021
@richard67
Copy link
Member

Back to pending due to requested changes.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35988.

@khu5h1
Copy link
Contributor Author

khu5h1 commented Nov 8, 2021

I also think that it is good to change admin to Administrator. I have committed the requested changes.:)

@wojsmol
Copy link
Contributor

wojsmol commented Nov 8, 2021

I have tested this item ✅ successfully on d0a87b3


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35988.

@wojsmol
Copy link
Contributor

wojsmol commented Nov 8, 2021

@richard67 PR in the other repo updated as requested.

@richard67
Copy link
Member

@wojsmol Thanks.

@manav014
Copy link

manav014 commented Nov 8, 2021

I have tested this item ✅ successfully on d0a87b3


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35988.

@richard67
Copy link
Member

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35988.

@khu5h1
Copy link
Contributor Author

khu5h1 commented Nov 8, 2021

Thank you Everyone for your comments and feedback. :)

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Nov 8, 2021
@richard67 richard67 removed the Updates Requested Indicates that this pull request needs an update from the author and should not be tested. label Nov 8, 2021
@bembelimen bembelimen merged commit 0ae41f9 into joomla:4.0-dev Nov 13, 2021
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Nov 13, 2021
@bembelimen
Copy link
Contributor

Thx

@bembelimen bembelimen added this to the Joomla 4.0.5 milestone Nov 13, 2021
Kostelano added a commit to JPathRu/localisation that referenced this pull request Nov 18, 2021
wilsonge pushed a commit to joomla-projects/joomla-browser that referenced this pull request Dec 5, 2021
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants