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

Disable cpanel and user menu when editing #4694

Merged
merged 10 commits into from Oct 17, 2014
Merged

Disable cpanel and user menu when editing #4694

merged 10 commits into from Oct 17, 2014

Conversation

dgrammatiko
Copy link
Contributor

Small UX improvement in ISIS template

What is it?

On ISIS when we are editing something (basically when we are in singular view) the main navigation becomes greyed out but cpanel and user menu are still visible. This PR hide them and removes their functionality.

Tests

Apply the patch and move around in the admin area

In pictures:

Normal view
screenshot 2014-10-16 13 34 35

Enter editing mode
screenshot 2014-10-16 16 59 22

@brianteeman
Copy link
Contributor

Sorry but I really dont like this

On 16 October 2014 11:44, Dimitri Grammatiko notifications@github.com
wrote:

Small UX improvement in ISIS template What is it?

On ISIS when we are editing something (basically when we are in singular
view) the main navigation becomes greyed out, unavailable but still
visible. This PR hide it altogether. If you cannot click it don’t show it!
Tests

Apply the patch and move around in the admin area
In pictures:

Normal view
[image: screenshot 2014-10-16 13 34 35]
https://cloud.githubusercontent.com/assets/3889375/4661252/56b0838c-5521-11e4-8ec0-545311a61367.png

Enter editing mode
[image: screenshot 2014-10-16 13 34 21]
https://cloud.githubusercontent.com/assets/3889375/4661254/5b831ec4-5521-11e4-84d4-5dba98de46a0.png

Scrolling on editing mode
[image: screenshot 2014-10-16 13 34 11]

https://cloud.githubusercontent.com/assets/3889375/4661260/6d587b30-5521-11e4-8ae2-6d6f9fea6b42.png

You can merge this Pull Request by running

git pull https://github.com/dgt41/joomla-cms isis_ux

Or view, comment on, or merge it at:

#4694
Commit Summary

  • Remove disabled navigation

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#4694.

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

@dgrammatiko
Copy link
Contributor Author

@brianteeman can you elaborate a little bit?

@brianteeman
Copy link
Contributor

To explain further - in the past we have had proposals to hide buttons when
they cant be used and they were all rejected as being confusing (I dont
agree but that was the consensus)

For the toolbar buttons the only thing people wanted was to grey them out
if they are disabled but no one came up with a good way to do that with
bootstrap.

So following those decisions it is correct that the menu is displayed as
greyed out when disabled

On 16 October 2014 12:14, Brian Teeman brian@teeman.net wrote:

Sorry but I really dont like this

On 16 October 2014 11:44, Dimitri Grammatiko notifications@github.com
wrote:

Small UX improvement in ISIS template What is it?

On ISIS when we are editing something (basically when we are in singular
view) the main navigation becomes greyed out, unavailable but still
visible. This PR hide it altogether. If you cannot click it don’t show it!
Tests

Apply the patch and move around in the admin area
In pictures:

Normal view
[image: screenshot 2014-10-16 13 34 35]
https://cloud.githubusercontent.com/assets/3889375/4661252/56b0838c-5521-11e4-8ec0-545311a61367.png

Enter editing mode
[image: screenshot 2014-10-16 13 34 21]
https://cloud.githubusercontent.com/assets/3889375/4661254/5b831ec4-5521-11e4-84d4-5dba98de46a0.png

Scrolling on editing mode
[image: screenshot 2014-10-16 13 34 11]

https://cloud.githubusercontent.com/assets/3889375/4661260/6d587b30-5521-11e4-8ae2-6d6f9fea6b42.png

You can merge this Pull Request by running

git pull https://github.com/dgt41/joomla-cms isis_ux

Or view, comment on, or merge it at:

#4694
Commit Summary

  • Remove disabled navigation

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#4694.

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

@dgrammatiko
Copy link
Contributor Author

This is not about hiding/disabling buttons, it’s about hiding the navigation. And from my point of view having something that is just taking space in the screen is not a good UX. Also another big thing is that when you enter the edit mode you should not have options to click on anything that will unintentionally exit the editing without first warning you so. As it is now the template when you are in editing mode you still have the joomla icon, the shortcut to the site and the cog icon which all of them, when clicked will not warn you that you will actually LOOSE everything you just altered.

@wdburgdorf
Copy link

I disagree with Brian that it is not confusing when buttons disappear completely, but totally agree with his "I really don't like this". PhpStorm (random example) works like that: The menu adjusts dynamically to available options. Driving me crazy. Looking for a menu item that was there a minute ago. If it's greyed out, I know it's probably for a good reason. If it's gone, I don't know if it's a bug, or if I'm looking in the wrong place. But since there's already a widely accepted consensus - do we need to discuss this here?

@dgrammatiko
Copy link
Contributor Author

@lausianne 3 things:
PHPstorm’s dynamically menus is totally different animal.
Before you disagree I thing you should try, you might be surprised!
Form follows function is a nice motto. In our case if there is no function there shouldn’t be any form.

@betweenbrain
Copy link
Contributor

This is an interesting idea. I do support the general concept of removing unusable elements from the page, but do have some concern about a consistent user experience and losing context.

I'm undecided at the moment, but would be interested to also hear the thoughts of folks like @CristinaSolana, @nternetinspired, @phproberto and others.

@wdburgdorf
Copy link

@dgt41 phpstorm is just one example of software where unusable elements disappear completely, and this causes problems.
What exactly should I try?
Disabled buttons totally do have a function. To know that there is a button, but currently (in the current context) you can't use it. The form is, it greyes out. If it has no function ever, then it should never be there ... of course.

@nternetinspired
Copy link
Contributor

My $0.02?

Primary navigation is a fundamental anchor for user experience. It's a fixed point that, like the constellations of stars above us, should be consistent, predictable and reliable if it is to be trusted and relied upon. I don't think it's ever good for users to hide it.

@infograf768
Copy link
Member

We already have a complex UI (parameters for example). "Losing context" (as Matt says) is not what we want.
-1 from me.

@Bakual
Copy link
Contributor

Bakual commented Oct 16, 2014

It also removes some active stuff (cpanel, user menu, frontpage link).

Judging from other comments, there is a big vote against this change. I'm closing it.

@Bakual Bakual closed this Oct 16, 2014
@dgrammatiko
Copy link
Contributor Author

I think I have to explain the purpose of this PR which is:
to DISABLE (by hiding navigation because this is the easiest approach here) links that are irrelevant when someone is editing something and thus PREVENTING THE LOSS of any alterations already done.
The other option will be to totally redesign back end so those links are not available in the first place.

@dgrammatiko
Copy link
Contributor Author

@Bakual Exactly those are the problem: when you are editing something those links should be as well HIDDEN (cpanel and user menu)

@dgrammatiko
Copy link
Contributor Author

@Bakual DISABLED i meant to write!

@infograf768
Copy link
Member

@dgt41
I agree it is desirable to grey these menus (Except the link to display front-end) when editing, but not hide them.

@dgrammatiko
Copy link
Contributor Author

@infograf768 I will try again… disabling and not hidding this time 😃

@brianteeman
Copy link
Contributor

very elegantly put @nternetinspired I will have to remember that one

@dgrammatiko
Copy link
Contributor Author

Ok this PR now DISABLES cpanel and user menu icons when in editing mode
@Bakual Can you open it again?

@Bakual Bakual reopened this Oct 16, 2014
@Bakual
Copy link
Contributor

Bakual commented Oct 16, 2014

Reopened as requested 😄

Can you update the screenshots in the description then?

@infograf768
Copy link
Member

@test
they are greyed, but they still work. :)

@brianteeman
Copy link
Contributor

Two good tests setting to RTC

@dgrammatiko
Copy link
Contributor Author

@brianteeman @infograf768 I just saw that the if statement in the scroll function is totally useless so I removed it. Sorry about that. Needs tests again

@brianteeman
Copy link
Contributor

Set back to pending

@brianteeman
Copy link
Contributor

Tests reverted

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

@dgrammatiko
Copy link
Contributor Author

Travis failure is not related to this

1) JAdministratorHelperTest::testFindOptionCanLoginAdminOptionSet
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'foo'
+’com_login’

@mbabker
Copy link
Contributor

mbabker commented Oct 16, 2014

Ya, that was my bad, it's fixed already.

@vanredweb
Copy link

The "Registration Date" have not splited to date and time yet.

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

@trangredweb
Copy link

In left menu in user manager, don't show button remove.

@tairedweb
Copy link

screen shot 2014-10-16 at 23 14 14
what about this ... it doesn't disable cpanel when i wrote new email :D

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

@phuredweb
Copy link

@test

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

@vanredweb
Copy link

missing the ID area in User name window

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

@brianteeman
Copy link
Contributor

@trangredweb This pull request is only disabling the cpanel and user menu on the views where the menubar is already disabled. If you think it should be disabled n other views please create a new issue

@vanredweb did you mean to post that here - it doesnt seem to be related to this isse

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

@trangredweb
Copy link

icon for button "Hide the sidebar" should change.

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

@infograf768
Copy link
Member

@test
Confirm it is working OK.
Note: we should indeed disable the menubar in com_messages edit

infograf768 added a commit that referenced this pull request Oct 17, 2014
Disable cpanel and user menu when editing
@infograf768 infograf768 merged commit 9428be4 into joomla:staging Oct 17, 2014
@mbabker mbabker added this to the Joomla! 3.3.7 milestone Oct 17, 2014
@dgrammatiko
Copy link
Contributor Author

@tairedweb please check #4788

@dgrammatiko
Copy link
Contributor Author

@infograf768 disable the menubar in com_messages edit #4788

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet