User tests: Successful: Unsuccessful:
To test that custom templates still work create a "form.php" in "templates/protostar//html/plg_twofactorauth_totp" (Google Auth) with an echo() or similar and
go to Frontend -> Edit User profile , select Google Autheticator and your form's content should be shown.
IMO, the plugin default layouts should be placed inside the plugin itself. Same for the language files though. But since this plugin is part of core, I am not sure if this applies here.
I also believe uninstall of the plugin should remove all its elements. Certainly, except the library classes it depends on.
@izharaazmi The language files can stay where they are I believe because all other core plugins have their language files in the admin language folder.
As for removing the library classes, that is a more difficult question because you cannot delete them when you uninstall 1 plugin because the other needs it nor can you uninstall it when you delete the 2nd plugin because theoretically you could install another authentication plugin that requires it. So I would say we can leave it as-is.
@roland-d I said except...
Certainly, except the library classes it depends on.
I meant to say – when uninstalling the plugin remove the default layouts too alongwith any plugin only files . And keep the library classes and any layout overrides which is obvious anyway. You too are saying the same.
IMO, the plugin default layouts should be placed inside the plugin itself.
IMO, /layouts
is not the best place for plugin's default layout files and uninstalling the plugin will not (and should not) remove it from there.
@izharaazmi Using the /layouts folder, we already have a layout there for the user profile plugin, so I believe it is the correct place.
As for the uninstallation, if I uninstall the profile plugin the layout is going to stay as well. The question should rather be should all Joomla core be rewritten to be fully uninstallable, perhaps but that is out-of-scope for this pull request.
Yeah, agreed on that. We'll talk over it some other day
@izharaazmi Using the /layouts folder, we already have a layout there for the user profile plugin, so I believe it is the correct place.
Just because that has been done already does not make it correct or even set precedent. The voting plugin and page navigation plugins both use tmpl/
directories for layouts. The stats plugin uses JLayout with a bunch of code inlined into it. So the existing practice for the majority is to keep the code contained in the extension, NOT use the global directory.
I am sure if we put it in the tmpl/ folder the comment would have been why we don't put it in the layouts folder :/
Well right now the plugin makes use of a tmpl/
folder so for B/C reasons whatever the logic for layout overrides is has to stay in place otherwise this cannot be accepted before 4.0 because it will cause HTML changes on sites implementing an override.
This is a large b/c break - we have a JPlugin helper method (https://github.com/joomla/joomla-cms/blob/staging/libraries/cms/plugin/helper.php#L38) that will keep b/c with the FOF layouts and whichever way we go with that extension specific layouts should 100% go in the extension directory and NOT the global layout directory. As Michael says see how roberto did the stats plugin
Imho, as long as 3rd parties can't install stuff into the layouts folder without custom installation scripts, our core extensions shouldn't do it as well.
The layouts folder should only contain JLayouts which are reusable across various extensions. Obviously that isn't the case today but it should be.
The proper place for an extension specific layout is the tmpl folder.
Or we make a move and let 3rd parties install their JLayouts into eg /layouts/com_foo/bar.php (similar to the media folder).
Cant say I am a fan of having a file called Totp.php and another called totp.php
@brianteeman Everything in libraries/cms
, libraries/joomla
, and libraries/legacy
should be all lowercased filenames since it uses Joomla's autoloader and it doesn't really account for or call for uppercased paths.
All of the namespaced code autoloaders pretty much call for title and camel cased paths. That's how PSR-0 and PSR-4 are defined and for Joomla's namespaced loader we follow the PSR-0 convention and most of the Composer dependencies are PSR-4.
So before this can be merged the path casing will have to be corrected.
@mbabker so my concern was correct?
On 11 August 2016 at 16:51, Michael Babker notifications@github.com wrote:
@brianteeman https://github.com/brianteeman Everything in libraries/cms,
libraries/joomla, and libraries/legacy should be all lowercased filenames
since it uses Joomla's autoloader and it doesn't really account for or call
for uppercased paths.All of the namespaced code autoloaders pretty much call for title and
camel cased paths. That's how PSR-0 and PSR-4 are defined and for Joomla's
namespaced loader we follow the PSR-0 convention and most of the Composer
dependencies are PSR-4.So before this can be merged the path casing will have to be corrected.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#11553 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8eLnUwS2bDRJNgmvNNxJifTSyF7xks5qe0UbgaJpZM4Jh5_z
.
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
Yes.
I can see files being moved to new locations and folders being moved but doesnt this have to be handled in the com_admin/script.php for sites updating
Eventually, yes. However, as pointed out the layout stuff will need to be adjusted for B/C so that will presumably move the files back to their original location and that should cancel out any need for updating the script.php file.
First of all thanks everyone for your input, in the recent changes I got the autoloading to work like some of you mentioned.
Due to your B/C concerns I reverted the changes to the templating but removed and replaced the FOF part that handles the custom template path
I have tested this item
applied PR and confirmed that both yubikey and google authenticator worked for site and admin login
Added small test instruction for custom templates.
Category | ⇒ | Libraries Plugins |
Status | New | ⇒ | Pending |
if the intention is to remove fof in joomla 4.0 this should be rebased to 4.0 branch
Why? this could already be done in J3 fine.
Is this still being worked on?
Status | Pending | ⇒ | Information Required |
Category | Libraries Plugins | ⇒ | Libraries Front End Plugins |
If this PR get no Response, it will be closed at 22th June 2017.
I kind of fail to see what is left to be done here, is it about the include
vs. include_once
debate?
@roland-d can you please have a Look at Status of this PR?
@franz-wohlkoenig As far as I can see this needs testing as @faynt0 already updated it with the staging branch.
@brianteeman can you please test again?
We need to update the Encrypt code with the latest version. change the branch to be against J4 and I promise I will get this merged!
Status | Information Required | ⇒ | Discussion |
Status | Discussion | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-08-23 14:11:29 |
Closed_By | ⇒ | wilsonge |
@izharaazmi thanks for pointing out, I changed the syntax.