User tests: Successful: Unsuccessful:
Pull Request for Issue #26022.
Remove all references of set(hidemainmenu)
outside configuration.php
. Now the behavior of the side menu is no more hard-coded.
Check if side panel appears where editor is used.
Side panel can be enable (hidden by default).
Side panel can be enable (hidden by default).
None?
Status | New | ⇒ | Pending |
Category | ⇒ | Unit Tests Repository Administration com_admin SQL Postgresql MS SQL |
Title |
|
“hidemainmenu” really was a bad name for the internal var, but that’s another story. In 3.x and earlier, when that is set it disables the menu links but keeps the menu visible. You’re changing the semantics of it here. IMO, it should keep the same semantics it always had (disable the menu links), then have another template level option to default show/hide the menu on edit views.
Remember, there is still the UX/workflow issue to sort out of not navigating out of the edit view using the toolbar items (close/cancel) leaving the item in a checked out state. Until that is addressed, I think it’s a bad idea to give the menu a visible state with all links active. So showing the menu is fine, but similar to now it should be inactive.
$ git remote add upstream https://github.com/joomla/joomla-cms.git
$ git checkout 4.0-dev-Sidebar
$ git fetch upstream 4.0-dev
$ git merge upstream/4.0-dev
$ git push origin 4.0-dev-Sidebar
Think that will do the trick
63c5804 makes too many changes then. Components are responsible for signaling that the menu should be in a limited state, so removing the line from all the views should not happen.
What you really should do is add a template param (not a global config option, because the sidebar is specific to the Atum template) and check that param as well as the input variable. What you have now is a global config variable (which implies it affects everything in Joomla) and code that reads that variable on all pages. Or, IMO just get rid of the show/hide sidebar feature because you really aren’t gaining much with it in the first place.
63c5804 makes too many changes then. Components are responsible for signaling that the menu should be in a limited state, so removing the line from all the views should not happen.
I do not think so. Having any value hard-coded/set in so many views is a very bad practice (not very MVC compliant)… This is at least error prone, and should be done in one place, not in so many files as it is the case…
What you really should do is add a template param (not a global config option, because the sidebar is specific to the Atum template) and check that param as well as the input variable. What you have now is a global config variable (which implies it affects everything in Joomla) and code that reads that variable on all pages.
Good point.
Or, IMO just get rid of the show/hide sidebar feature because you really aren’t gaining much with it in the first place.
I disagree… I really think that a static left panel + something like #4773 / #5815 will be very simple, but good improvement.
The problem with your commit and your proposed change is you are changing the template behavior from basically only putting the menu in a limited functionality state on edit views by default to potentially putting it in a limited functionality state on all pages. That's not what you want here. You might view that line being repeated in 20 components as an error prone hardcoded behavior, but that is exactly how a modular MVC based system should function. You are removing control of a system behavior from the component, where it has existed for 14 years without an issue.
The component should still be responsible for signaling "the menu should be in a limited function state" until whatever comes out of the newest iteration of #4773 or #5815 is actually implemented. And once those are done, then honestly the line you've removed from all the component views can go away completely.
You want to be able to set the sidebar to always be expanded. That is a new template parameter and adjusting the template's index.php
file to account for that parameter. That is not changing how an internal system variable behaves.
The problem with your commit and your proposed change is you are changing the template behavior from basically only putting the menu in a limited functionality state on edit views by default to potentially putting it in a limited functionality state on all pages. That's not what you want here.
You might view that line being repeated in 20 components as an error prone hardcoded behavior, but that is exactly how a modular MVC based system should function. You are removing control of a system behavior from the component, where it has existed for 14 years without an issue.
These lines repeated in several component's view (JFactory::getApplication()->input->set('hidemainmenu', true);
and JFactory::getApplication()->input->set('hidemainmenu', 1);
and other variants) are definetly not MVC.
In MVC (in its pristine form, like Trygve Reenskaug), the view is so stupid that it can only:
Even after 14 years, bad decisions/design can be fix, as well as my Joomla's nooby trying.
You're right that the view isn't the right place to be setting that request var and that it'd be better suited in a controller, but my point is it is a behavior that is triggered by the component and I don't think this should be taken out of the component's scope to decide UNTIL Joomla can be made to work so that leaving an edit screen by any means other than the toolbar buttons doesn't leave unwelcome side effects. This is why I'm saying you're better off right now with a parameter in the template that sets the sidebar state, or just flat out removing the logic to show/hide the sidebar by default and just have it always default to expanded open (and if someone is really adamant about having this functionality, then I'd honestly change it so it's 100% JavaScript driven and use localStorage
to store the state of the sidebar).
With the change you've made, as is you are defaulting the menu to be in the limited functionality state for the entire backend interface. This is creating the side effect of there not being a sidebar at all anywhere.
Again, there are two separate actionable items to be worked on:
You're trying to tackle both in one go with an over-eager (and massively B/C breaking) change.
You're trying to tackle both in one go with an over-eager (and massively B/C breaking) change.
I do not think so. As I have written earlier, I want to
Maybe I am wrong, but seems that, at least, @brianteeman and @dgrammatiko would prefer to keep things unchanged (even if I do not understand the later; how a toggle menu would take 1/3 of our 16:9-16:10-21:9 screen space).
So what I intend to do is to propose something which will please everyone. Then work on the UX issue. But maybe I am wrong. I'll take a look to the UX issue in depth, and maybe I will start there.
Only speaking for myself but I believe you are confusing your idea of a permanent but disabled sidebar with a proposal to allow unrestricted use of the menu
I think we're talking past each other right now. Let me try to explain here.
The lines you removed from the components are the lines that signal to the system (mainly the admin menu module and the template) that the menu should be in a limited functionality state. In 3.x and earlier, this basically only disabled the links in the menu module while still rendering the menus; with 4.0 (and specifically the Atum template) this is also telling Joomla to not render the sidebar on a page at all.
The issue people are presenting is that the sidebar does not render at all on edit views. Opinions on that aside, if the desired state is to have the sidebar render on edit views, then you should back out any code that explicitly blocks rendering the sidebar without touching any other code, which looks to be removing the if conditional around this block of code.
Now, for the issues with your changes in 63c5804. You have removed the signal from components that the menu should be in a limited functionality state and have changed the admin menu module and Atum template to use a global configuration parameter, defaulting to true which means that by default the menu on all pages is in the limited functionality state, which means with Atum there is no sidebar rendered at all. In other words, your change proposal actually doesn't work in the way you seem to intend it to and it actually leaves the backend in a broken state because without a manual edit to configuration.php
you can't get a sidebar enabled.
So starting from a clean 4.0-dev branch, these are the changes you need to make to deal strictly with the fact that the sidebar does not render at all on edit pages:
That will address the "sidebar does not show at all" issue without intermixing any other issues into the fix.
(And for clarity, the UX related issues deal with navigating out of edit views without using the toolbar, specifically any close/cancel type button, which causes the item to remain in a checked out state, which is why the menus go into a disabled state in the first place to try and steer users away from just clicking any random thing on the page; until that workflow issue is addressed the menu should not be rendered in an enabled state on edit pages, but again that is a totally separate issue to deciding whether the sidebar should render on edit pages or not)
@mbabker Thanks for you explanation. I did not notice that the variable was responsible of displaying and hidding the menu in the entire Joomla scope, as the name is hidemainmenu.
What we would say in Java/C# world, is that this name is lying… This name sucks :s
A better name, for such a scope would have been something like disableSidePanel
, shutdownSidePanel
, or unloadSidePanel
. Yet, this variable is responsible for two things, so it still weird…
I use to think that this variable was only intended to deal with editing, where there are forms. Not in all the website. Why should it be otherwise? For what purpose? It is illogical for such a variable to cover such a scope. I cannot see any valuable reason.
And for you and @brianteeman.
What I want is
A better name, for such a scope would have been something like
disableSidePanel
,shutdownSidePanel
, orunloadSidePanel
. Yet, this variable is responsible for two things, so it still weird…
The input variable "hidemainmenu" has existed long before the Atum template and 4.0, it dates back to Joomla 1.5 where it hid the "submenu" module position, and continued like this up until 2.5 (sorry, I don't have a PHP platform old enough to load up a 1.5 or 2.5 site to see what the submenu position actually was, and nobody could pay me enough to painfully find a PHP 5.3 or 5.4 platform to actually do that). So changing the name of it at this point, confusing as it might be now, would be a B/C break without much gain (and no, I don't consider renaming things for the sake of renaming to be a strong reason for a B/C break myself).
I use to think that this variable was only intended to deal with editing, where there are forms. Not in all the website. Why should it be otherwise? For what purpose? It is illogical for such a variable to cover such a scope. I cannot see any valuable reason.
In core uses, it is only used on edit views to basically turn the menu module into a list of text items without any actionable links (since HTML lacks a proper disabled attribute for <a>
elements). It is only with this design of the template that the input var signaling to disable the menu also turned into something that completely hides a section of the template. Theoretically, extensions could also trigger this behavior in their own contexts if they wanted to disable the main navigation in a component view for some reason (a component which has some kind of wizard would probably be a good example of a valid use of this in a non-edit context, Akeeba Backup also triggers this while a backup is running). I'm not tied to either way of handling things (completely hiding the thing or rendering the thing in a limited state), but I do think it would be best to keep treating that variable using the in-practice behavior it seems to have had through the 3.x lifetime, which was to disable the links in the menu module while continuing to render the module (which, in the current template, is the sidebar that is now hidden).
You know what, it's too much for me.
With all due respect, Joomla is obviously:
(JFactory::getApplication()->input->set('hidemainmenu', true);
), and leads to tidely coupled components, the opposite of pristine MVC which is about of separation of concern…Now I understand why I found some Joomla's people psychorigid. This rigid system is so difficult to change, that it leads to psychorigidity…
I give up, it will be too much involving and for a poor result.
Anyway, thanks for all your help, patience and advice.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2019-08-28 05:54:31 |
Closed_By | ⇒ | vintzl | |
Labels |
Added:
?
?
|
Why are there so many changes on this PR?
Looks like to need to fetch from upstream and merge into your branch...