joomla / backend-template

backend template for Joomla 4 working area
GNU General Public License v2.0
13 stars 23 forks source link

Status Modules - review #333

Closed brianteeman closed 5 years ago

brianteeman commented 5 years ago

I started off with the intention of doing a pull request to fix some really obvious errors in the new admin status modules such as creation dates, nonsense strings, non-existent classes, broken accessibility

However I cant do that as I cant see the reason that they were created. In the main repo they are all in one module and with accessible and semantic navigation markup. Now we have multiple modules - why? What was the benefit/reason? What was the logic of putting them in individual modules.

As I dug in to the modules further they are a mess of copy/paste code with lots of duplication of unused code in each module and don't get me started on the clusterf* that has happened with the multilingual status module - I am still crying at that one.

brianteeman commented 5 years ago

oh boy - didnt realise that mod_version had been rewritten as well. If you are not going to use the helper to get the version then why is it still there and why add new code to get the version number when we already have core code for that and surely there us a better way to get the product name than creating a new language string.

and can anyone explain why all the namespacing has been added https://github.com/joomla/backend-template/commit/fee7a602bb6287925e10db1f1f531a0d265c428c#diff-928be8321e1fb842acc32d2e1eca80df

brianteeman commented 5 years ago

If I knew why they had been separated into different modules etc then I can try and fix them. If no one knows (or its a silly reason) then I can try and put them back together.

bembelimen commented 5 years ago

The main reason was to be able to split them in the mobile dropdown and to reuse them like the frontend link in the login page and the normal header.

brianteeman commented 5 years ago

the mobile dropup can be done without splitting them afaict as for the login page link does that even have to be a module - its only a link after all.

I just think its over engineered and over complex to split them.

If you really want them split then I can work on fixing them (basically deleting redundant code and fixing rhe autoload of the mm status module) or I can work at putting them back into one module as they were before and style the mobile dropup as it looks now

I should also poiint out that the current code is not as accessible as it was.

bembelimen commented 5 years ago

I personaly would prefer to have them splitted and yes, every help in cleaning up and make them accessible is very appreciated.

brianteeman commented 5 years ago

OK. I will do this Sunday (i'm out all day today).

brianteeman commented 5 years ago

Done #360