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

[5.2] Enable automatic title for admin modules #36954

Open
wants to merge 17 commits into
base: 5.2-dev
Choose a base branch
from

Conversation

wojsmol
Copy link
Contributor

@wojsmol wojsmol commented Feb 6, 2022

Pull Request for Issue #35587 (partly)
Related to #21330

Summary of Changes

Make sure automatic title is enabled by default like we had it in 2.5

Testing Instructions

Install package build for this PR by dron and confirm that the setting Automatic Title is enabled for Popular Articles, Recently Added Articles, Logged-in Users and Latest Actions.

Actual result BEFORE applying this Pull Request

The setting is disabled and just the original title is displayed.

Expected result AFTER applying this Pull Request

The setting is enabled and shows the generated title instead of the default.

Documentation Changes Required

@wojsmol wojsmol changed the title Enable automatic title for admin modules [4.2] Enable automatic title for admin modules Feb 6, 2022
@richard67
Copy link
Member

@wojsmol Could you refer to the right issue #35587 instead of the duplicate #35820 ? Thanks in advance.

@wojsmol
Copy link
Contributor Author

wojsmol commented Feb 6, 2022

@richard67 Issue number updated.

@wojsmol
Copy link
Contributor Author

wojsmol commented Feb 6, 2022

@richard67 This PR fixes #35587 only partially for modules witch have automatic title support.

@richard67
Copy link
Member

@richard67 This PR fixes #35587 only partially for modules witch have automatic title support.

@wojsmol Then you should have written something like „Pull request for issue #35587 (partly)“ at the top of your PR. It causes us additional work to sort out which issues to close or not to close it people don’t properly refer to issues in their PRs.

@wojsmol
Copy link
Contributor Author

wojsmol commented Feb 6, 2022

@richard67 PR description fixed

@richard67
Copy link
Member

Thanks.

@brianteeman
Copy link
Contributor

#21330

@zero-24
Copy link
Contributor

zero-24 commented Feb 6, 2022

#21330

Yes thats why we first asked the 4.2 release leads and they signaled us that such a PR would be accepted into 4.2 to improve non-english backends. This PR is only changing it for new installations so existing installations are still working as they do now. :)

@brianteeman
Copy link
Contributor

#21330

Yes thats why we first asked the 4.2 release leads and they signaled us that such a PR would be accepted into 4.2 to improve non-english backends. This PR is only changing it for new installations so existing installations are still working as they do now. :)

And someone was to know that how? Mind-reading?

@brianteeman
Copy link
Contributor

This PR is incomplete. There is zero point in just having a few modules in the admin language. If you're going to do this then do it properly and make sure that all the modules etc have automatic titles.

@brianteeman
Copy link
Contributor

image

image

@brianteeman
Copy link
Contributor

Also please move the switch to use the automatic titles to the main tab. Otherwise we will have users (once again) who dont understand why none of the changes they make to the module titles work.

@brianteeman
Copy link
Contributor

I have tested this item 🔴 unsuccessfully on 4666c95

This breaks(?) all existing sites.

On an existing site the installation sql did not add the default value of automatic_title=0 which was not really a problem as it matched the default settings in the module xml

However this PR changes all of that.

On an existing site - apply this PR.
There is no visible change and modules still use the natural title
Open any article for editing and you will be confused as it will show that automatic titles are being used - they're not
Open any article for editing and change the title and save - now you will be confused as your module is neither using the original title or the title you just set. Instead it will use the automatic title.


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

@wojsmol
Copy link
Contributor Author

wojsmol commented Feb 7, 2022

@brianteeman Issue reported in #36954 (comment) was fixed in 799a64a

@brianteeman
Copy link
Contributor

The pr is still incomplete as shown in the screenshots

@wojsmol
Copy link
Contributor Author

wojsmol commented Feb 18, 2022

Restart of drone test is required.

@brianteeman
Copy link
Contributor

Drone issue is my fault. Will need #37092 to be merged to resolve that

@joomla-cms-bot joomla-cms-bot added the Language Change This is for Translators label May 4, 2022
@wojsmol
Copy link
Contributor Author

wojsmol commented May 25, 2022

@brianteeman I just merged latests 4.2-devf into branch of this PR but tests are still failing.

@zero-24
Copy link
Contributor

zero-24 commented May 26, 2022

yes looks like the new sampledata helper is not loaded in the module yet. i was sure i have tested that too but looks like i have not.

@richard67
Copy link
Member

richard67 commented May 26, 2022

yes looks like the new sampledata helper is not loaded in the module yet. i was sure i have tested that too but looks like i have not.

The log shows that installation works well, but then it fails to unpublish the statistics module. Possibly the tests need to be adjusted to a changed module title or something like that? If that is the case, all human tests will work and only the automated tests fail as long as not adapted.

@brianteeman
Copy link
Contributor

This PR is still incomplete

@wojsmol
Copy link
Contributor Author

wojsmol commented Jun 9, 2022

@brianteeman Can you elaborate what is missing?

@brianteeman
Copy link
Contributor

same as I reported before

image

@brianteeman
Copy link
Contributor

the main problem however with this PR is it is completely non intuitive.

The principle of the pr is a good one but the implementation is wrong. If you look elsehwere in Joomla 4 where we have translatable titles it is much clearer.

As it is right now there is no way that a new user would understand that the title the title of the module Latest Articles can not be changed unless they also turn off the automatic title setting. It just looks like a bad bug that the title never changes no matter what you put in the module.

Take a look at workflows for an example of how it could be done
image

@Quy Quy added the Updates Requested Indicates that this pull request needs an update from the author and should not be tested. label Jun 11, 2022
@joomla-bot
Copy link
Contributor

This pull requests has been automatically converted to the PSR-12 coding standard.

@HLeithner HLeithner removed the psr12 label Oct 23, 2022
@Hackwar Hackwar added the Feature label Apr 6, 2023
@HLeithner HLeithner changed the base branch from 4.2-dev to 5.0-dev May 2, 2023 16:42
@HLeithner
Copy link
Member

This pull request has been automatically rebased to 5.0-dev.

@HLeithner
Copy link
Member

This pull request has been automatically rebased to 5.1-dev.

@HLeithner HLeithner changed the base branch from 5.0-dev to 5.1-dev September 30, 2023 22:52
@HLeithner HLeithner changed the base branch from 5.1-dev to 5.2-dev April 24, 2024 09:09
@HLeithner
Copy link
Member

This pull request has been automatically rebased to 5.2-dev.

@HLeithner HLeithner changed the title [4.2] Enable automatic title for admin modules [5.2] Enable automatic title for admin modules Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Language Change This is for Translators PR-5.0-dev PR-5.2-dev Updates Requested Indicates that this pull request needs an update from the author and should not be tested.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants