User tests: Successful: Unsuccessful:
Update to UI/UX and language file naming conventions.
This is an improvement proposed to the collapsible sidebar introduced since 3.3.7-dev by @roland-d.
It is based on UI proposal by @dbhurley : #4450
And on ideas from many contributors!
In this PR, a sliding effect is added to the sidebar to improve UX, and UI was changed to a column styling.
The sidebar is LTR/RTL ready and is displayed on screen with a min-width of 768px.
Screens under 767px wide use the sidebar as it is already displayed in 3.3.6.
Future improvement with mobile devices could be proposed in another PR.
Intructions for testing :
Comments and tips are welcomed!
Cyril
Labels |
Added:
?
|
Category | ⇒ | Templates (admin) UI/UX |
Thank you @yanivRozenman
Could you revert and patch again ?
I've just changed compression of core.js for Travis to succeed. ;-)
@losedk i've updated to fix your issue. Could you test again if you don't have again the inline scroll (not needed in fact...)
@losedk and @roland-d i have added back the same gradient as for the toolbar background. Thanks for your feedback about it, i was not sure it was best to have the gradient or not. ;-)
Here is my thought on this:
1: You don't ever want to make the menu fully disappear. It creates a usability problem. So replacing the text with simple icons on the left that are still clickable will resolve this issue.
2: I would change the X to a arrow left.
@spignataro I think your first proposal is not feasible in current joomla code (and 3rd party as well). The menus don’t have an icon attached, so we either have to come up with a very clever text to icon thing or we introduce a new method. Introducing a new method should also have a fallback etc.. Although the idea is indeed very good, I’ll say again it’s not feasible right now.
Also I like X more than a left arrow, because nowadays with all the mobiles and tablets out there, the X icon, universally, indicates close, which is exactly what this button will do.
I agree with @dgt41 about 1, and about 2, in this design, the X is the best choice.
But i've got maybe an idea, to change toggle button in a fixed left position (LTR), and would like your opinion on it :
Don't know if it could be better or not, for user experience in usability...?
@dgt41: I understand the reasoning now - thanks for explaining this. Still further on the usability - if icons are not a feasible choice I would still argue that a X is not a suitable choice for a sidebar vs a MENU which it is more associated with in mobile. Right now if you are in mobile the sidebar disappears completely and is not even anywhere to be found or at least I have yet been able to find it. That might be something to also address with this update as well.
@JoomliC: I actually like this - I would make one suggestion - can we move the blue bar to the right of the filters so that the user knows that it will collapse the left sidebar. I would even venture to possibly using a grey color when on the right - but I think we can play with colors once we have a working mechanism.
Again those are my thoughts.
@JoomliC like the idea! But agree with @spignataro, the blue bar should be moved to the right
Alas, this breaks totally com_localise as our filters are not set up to be included in the toggle.
Tried to find a solution in our custom submenu layout, but it has changed from ul to <div>
, so blocked for the moment
@infograf768 At the moment the collapsible sidebar has never gone into a release so we can make b/c breaking changes to it.
is com_localise part of 3.4 @infograf768 ?
No. It's a Joomla supported component that we're building. It won't be part of the CMS
Ahh okay
@infograf768 i have reviewed my code, which one is working again with com_localise, by setting the jquery animate on #j-sidebar-container.
So, i will update this PR, but i need everyone's opinion about the button icon?
As it is currently (just arrow and closed icons) or do i integrate the blue bar in the update ?
@losedk and @spignataro for the fixed position on left, i was inspired by adobe softwares.
And in fact, i found it nice to be able to open/close without having to move the cursor. ;-)
So, if blue bar, position fixed on left, or moving with the sidebar ? And colors ?
You are doing things right imho.
The issue for com_localise is that I can't figure a new hack to make it work correctly as our filters are not included in the sidebar by JHtmlSidebar::addFilter()
When submenu used <ul>
I could manage by changing our default_filter.php files and adding a specific layout without the 2 final closing </div>
, but no anymore.
See joomla-projects/com_localise@7eff414
and
https://github.com/joomla-projects/com_localise/tree/d6f9ea96e836100d3fff256911a11b3273f1e1f5
@JoomliC I like the second proposal as well, but personally I vote for the first (Its hard to make a decision on peoples tastes). One proposal if the second iteration win: associate the color with the color of the header by adding the related css @ line 95 on index.php
<!-- Template header color -->
<?php if ($this->params->get('headerColor')) : ?>
<style type="text/css">
.header {
background: <?php echo $this->params->get('headerColor'); ?>;
}
#j-toggle-sidebar-button {
color: <?php echo $this->params->get('headerColor'); ?>;
}
</style>
<?php endif; ?>
@infograf768 you won't have to update com_localise code, when i will update this PR, it will be working fine with com_localise 3.2.1 ;-)
@dgt41 Yes, love idea of a color option in template, but i think this could be another PR, as there's already a Sidebar color option, which one could confuse.
@spignataro it's normal that filters disapear on small devices, and you won't see submenus if not any. Test with com_content ;-)
MY QUESTION : Do i update the PR with icon-arrow-right-2 / icon-cancel as it is in the current PR, or do i introduce the blue bar ?
Our last release is now 4.0.7
https://github.com/joomla-projects/com_localise/releases
@infograf768 i had installed 3.2.1 with Install from web...
So, with 4.0.7-dev, not working (just the close icon and no animate but we can see all the submenus and filters)
With 4.0.6-dev, the sidebar is working fine! Just he main content is smaller than it should be.
I will try to see how to solve this...
3.2.1 com_localise is an old stuff unusable. I will have to get it out of joomlacode soonish
@infograf768 So, with my new code, no need for com_localise to override the layout (that's why i got an issue) and no need so of the 2 closing div in default_filter.php
@infograf768 do you have a preference for the icon-arrow-right-2 / icon-cancel as it is in the current PR, or do i introduce the blue bar ? (it is to update this PR, and so that to able you to view the effect on com_localise (in fact, a good one, as no needs of special override)
If we go for the blue sidebar, I would say have it fixed left (or right for RTL). As for the icons, I have no preference, your first pick is good enough for me.
I've just updated the PR with fix for alert message, and better integration.
I think now, we have a working mechanism ;-)
I've let the cancel/arrow design for testing first.
@infograf768 now, with no override of the layout in com_localise, it won't be an issue anymore. ;-)
@roland-d the blue bar seems to have different opinions on it... If blue bar, i'm for a fixed position too. But as @dgt41 said : " Its hard to make a decision on peoples tastes ". Is the blue bar not too "skilled" user accessible ?
My suggestion would be to think from a community view and user usability. Simplicity is always efficient but difficult to reach!
Perfect. For com_localise we just have to release a new version with the core submenu in the layout folder and not the custom one.
The issue comes from
if (height_jsidebar < height_doc)
{
jQuery('#j-sidebar-container').height(height_doc-78);
}
and, if we delete that part, we get a height: 100%;
for the #j-sidebar-container
Taking off both solves the issue here, (except for com_localise where the script becomes unresponsive _when debug is on, but that is due to other issues with debug)
@infograf768 Thanks for this report! ;-)
I have delete that code, as in fact, it was a bad idea... My purpose was to extend height when main content higher than screen view, but gave more issue than the expected better design.
"when debug is on, the sidebar covers the left part of the debug" I was not able to fix it in sidebar, as the system-debug div is not in the main container-fluid...
@blueforce could you revert/clear cache/patch again to see if your issue is solved ? (i can't reproduce it, nor chrome or firefox...)
Test by taking off
+ height: 100%;
in #j-sidebar-container
@JoomliC yes, the cache is cleaned
it only appears, when I scroll down, when it's possible (e.g. a long User or item list)
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5141.
@infograf768 i've just updated, and removed the fixed position for button, and after testing with no height, the filters are not cut.
Could you test this new update ?
@blueforce could you test again? I've removed the fixed position, which was maybe a reason for your issue... i hope! ;-)
Yep, the problem is solved now.
It looks good, great job!
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5141.
@blueforce Thanks for testing! Good news that's your issue is solved ;-)
Ok now for the dropdowns. Remains, for me, only the issue of the debug when the manager is almost empty and sidebar contains lots of stuff (as in Languages Manager=> Contents tab when only one content language there.
It is true though that when debug is on in such a case, we just have to close the sidebar.
@JoomliC Works great, well done. Indeed, in that one particular case the sidebar just has to be closed unless we are willing to move debugging to the main body. It isn't really a page that requires a lot of debugging if you ask me, when it does, closing the sidebar is a little price to pay :)
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5141.
@test success. As I mentioned on a previous comment for the shake of consistency, the icons: left arrow and X actually are buttons and will be nice if the color followed the bootstrap button link logic.
Two minor changes as I pointed here:
#5141 (comment)
#5141 (comment)
Will make the clickable button consistent with all the clickable links on the page! Just a proposal...
@dgt41 Yes! Added, and removed by error in last update. Back again in template.less ;-)
@infograf768 & @roland-d I agree that move of debug to main body could help. Could be done later ?
@roland-d I have a very quick solution for debug but needs some css love:
replace in /plugins/system/debug/debug.php (line 285)
echo str_replace('</body>', implode('', $html) . '</body>', $contents);
with
echo str_replace('</body>', implode('', $html) . '</body>', $contents);
if (JFactory::getApplication()->isAdmin() && JFactory::getApplication()->getTemplate() == 'isis')
{
echo "<script>
var newParent = document.getElementById('j-main-container');
var oldParent = document.getElementById('system-debug');
while (oldParent.childNodes.length > 0) {
newParent.appendChild(oldParent.childNodes[0]);
}
</script>";
}
Noooo pls no template specific stuff in plugins
@dgt41 Great! And... in fact... really nicer design for debugger (clearer) ;-)
This could be a nice separate PR !
@dgt41 as i'm not at ease with generatecss.php, i will give it a try later, but love the idea of the less possibilities i didn't know before! Thank you (and for all your work since the first PR on the toggle sidebar!)
The question is should i let the icons in grey (as currently) or follow the recommendation (User friendly one) from @dgt41 to "make the clickable button consistent with all the clickable links on the page" ?
This PR is almost ready, so opinions on colors are welcome! (and @roland-d your opinion is important too as you're the "father" of this work ;-) )
My opinion : blue link type icons are more visible, and easier to find
I haven't tested this so dunno which is which. But prefer the ones on the
right
On 21 November 2014 23:48, Cyril Rezé notifications@github.com wrote:
@dgt41 https://github.com/dgt41 Great! And... in fact... really nicer
design for debugger (clearer) ;-)
This could be a nice separate PR !@dgt41 https://github.com/dgt41 as i'm not at ease with
generatecss.php, i will give it a try later, but love the idea of the less
possibilities i didn't know before! Thank you (and for all your work since
the first PR on the toggle sidebar!)The question is should i let the icons in grey (as currently) or follow
the recommendation (User friendly one) from @dgt41
https://github.com/dgt41 to "make the clickable button consistent with
all the clickable links on the page" ?
[image: color_sidebar_icons]
https://cloud.githubusercontent.com/assets/2385058/5151620/072fdf4c-71e1-11e4-95fa-af2cc55ab43e.jpg
This PR is almost ready, so opinions on colors are welcome! ;-)My opinion : blue link type icons are more visible, and easier to find [image:
]—
Reply to this email directly or view it on GitHub
#5141 (comment).
Ready!
Good Tests and good night ;-)
Ran generatecss and looked a bit further in the rtl less/css
As explained above, the rtl less imports first template.less and then bootsrap-rtl.less, then anything added to that in the rtl.less, then the css file is generated.
This means that any details which is set to right in the ltr template.less,
for example:
right: -16.5%;
has to be set first to right: 0;
for rtl
in the rtl less AND another one added left: -16.5%;
(if necessary)
Same for border-right, etc.
If anyone feels that can cope with the styling of debug, here is the code needed for isis/index.php (put it before </body>
:
<?php
if (JFactory::getApplication()->get('debug'))
{
echo "<script>
var oldParent = document.getElementById('system-debug');
var newParent = document.getElementById('j-main-container’);
if (newParent)
{
while (oldParent.childNodes.length > 0) {
newParent.appendChild(oldParent.childNodes[0]);
}
}
</script>";
}
?>
@JoomliC @roland-d One more change here this time at index.php on isis folder
At Line 119 add these 3 lines:
#j-toggle-sidebar-button {
color: <?php echo $this->params->get('linkColor'); ?>;
}
This will make sure that our color is always in sync with the chosen ones in the template!
Also @JoomliC as @infograf768 also stated above you have to sort out the less files so generatecss works flawlessly!
@roland-d The regeneration part is mandatory otherwise the next one will touch isis will break it! So not ready yet
I am online Skype: velocity-gr
@JoomliC are you online? got your files ready
Try this https://dl.dropboxusercontent.com/u/55769984/less_css.zip
Sorry @dgt41 just see your message... and no skype installed back (i've uninstalled it some months ago after a bug, but as i'm never using it... :-D (i will maybe later, couls be useful sometimes)).
I've just updated the template-rtl.less and thanks to your help, i'm able to run generatecss.php on my dev local!
You were right, there was an issue, and after testing my last changes, it's working fine for me.
Will have a look at your files now!
@dgt41 i've tested with your file, but an issue as before in RTL after generatecss.php.
But if you test the PR as it is now after my last update of less, you should not have any issue after generatecss.php. I have follow @infograf768 suggestion ( Merci! ), and it's doing the job for template-rtl.css (with addition of left: auto or right : auto and a border-right: 0 in template-rtl.less)
Also @JoomliC as @infograf768 also stated above you have to sort out the less files so generatecss works flawlessly!
@roland-d The regeneration part is mandatory otherwise the next one will touch isis will break it! So not ready yet
@dgt41 The duplicates seems not an issue with the less files, but with generatecss.php and rtl.css generation.
When looking for example at "container-logo" class, it is too duplicated in the rtl file, as it was already before this PR.
On this PR not yet having updated the template-rtl.css because i wonder how i can generate it on gitHub... But it works as expected, and if we regenerate css using generatecss.php, there is no issue in functionnality.
I've now updated with template-rtl.css generated from generatecss.php
To be tested, but i think now it's okay! (and we should not wait christmas to go ! ;-) )
@JoomliC @infograf768 Check #5175
@dgt41 @infograf768 @roland-d i've updated this PR again, and i think it's working perfectly with debug mode now!
Just a few lines added in core.js to do the job depending on sidebar / content height.
So, this PR is to be tested again, but i think now we are near the goal!
@test all good for m. thanks
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5141.
Thank you @brianteeman !
I'm sorry, just a little change for mobile devices under 768px wide, to display debug box...
(no changes for desktop screen or laptop)
Status | Pending | ⇒ | Ready to Commit |
Moving this back to RTC since we have 2 successful tests after the latest changes.
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5141.
Sorry, not sure this can go RTC.
Please look at the behavior of the sidebar when one reduces and enlarges the window.
https://www.dropbox.com/s/rk3yvvqt5ipm0nf/sidebarissue.mp4?dl=0
Firefox-Macintosh
The flickering is not the issue for me (my screencapture app I guess). The issue is that when one enlarges again the window, the closed sidebar icon gets hidden behind the Search field and the sidebar does start to show to the left of the screen.
Sorry, I thought it was the flickering, wrong alarm and also wrong PR that one that I referenced. More coffee...
Thank you for report @infograf768 !
Yes, as @dgt41 i thought first of the flickering... ;-)
I found the reason of this bug : the animate function.
I will try to fix it, as using the sidebar width in px is not responsive.
I'm now searching a way of using a responsive solution.
use .025em;
or similar?
use .025em; or similar?
No, i'm converting in core.js width in percentage, and it's working. I'm doing the same for debug box, and it should be okay ;-)
var $sidebar_width = (($sidebar.width() / content_width) * 100)+'%';
@infograf768 Good find. I can reproduce that problem here as well.
Labels |
Added:
?
|
@infograf768 @dgt41 @roland-d Updated, and ready for testing resizing...! ;-)
Thank you!
Did we drop the 100% height again? I my opinion it looked better before
@test OK
The problem @roland-d mentioned above I guess happens when the toolbar buttons occupy two lines, instead of one.
There is a minor performance thing though with the recalculations of the width/height in javascript. It consumes much CPU power and in slow machines this may become sluggish. But I guess is not a problem and if this can be done with css3… No it CAN’T because we still support IE8.
Anyhow I just wanted to mention that there is a price for the nice effects.
@roland-d @dgt41 this was already moving main content between 768px / 969px, it's not relative to this PR. ;-)
@losedk yes because 100% was not debug mode friendly, and not working properly when long list of items.
@dgt41 of course, would be easier with css3 off-canvass effect, but we have to support IE8 (always a big debate...)
Do you have an idea to improve it?
IE8 is one of our supported browsers in the 3.x series and it should function properly. We added an additional beta release leading into 3.3 over IE8 compatibility issues (http://www.joomla.org/announcements/release-news/5543-joomla-3-3-beta-3-released.html) so we should do our best to make sure it's working now.
Didn't read or infer that at all. My main point was to say just make sure that IE8 works.
We are on the same page here!
@dgt41 using a px width is not the best option, in my opinion, for a responsive behaviour...
About performance, due to .width() usage, i've found useful info there : http://blog.jquery.com/2012/08/16/jquery-1-8-box-sizing-width-csswidth-and-outerwidth/
Could you test performance with this code :
https://gist.github.com/JoomliC/8b1b585d608bfbca9189
Thanks!
PR solved here the issue I described above.
nice job
but...
maybe I will be a boring ...
I think would be better put all JS code that used here to the template specific file isis/js/template.js
... as it just a styling thing.
core.js
often used on the frontend and this part of code useless there
I disagree with the layout file as you proposed before because inline JS. I agree about putting it in a separate template specific JS file tho
@dgt41 don't be sorry, your help is really appreciated, and i agree that UX performance is important!
YES your updated script code is way better in terms of performance. Almost everything is over 60FPS.
So please push that code!
Updated! ;-)
I agree about putting it in a separate template specific JS file tho
@wilsonge Should we move the Joomla.toggleSidebar() function to template.js or a new specific js file ? sidebar.js ?
@Fedik @wilsonge @JoomliC I am afraid that putting the code to a template related file is not an option as it will break B/C. Let’s say somebody got their own admin template, if they don’t have the script loaded in core.js their template will be broken. Also Hathor will be broken. I guess for front end we should keep shaving core.js
toggleSidebar
is new feature, that will be available starting from 3.4 ... or?
then I see no problem
I also think that should be no problem move to template.js
next code:
jQuery(function()
{
Joomla.toggleSidebar(true);
});
that currently in submenu.php
Meh. Maybe just leave it in core.js for now and we can reconsider for a further PR moving it elsewhere. Like I just want to make sure this much improved design makes it into 3.4 :)
Meh. Maybe just leave it in core.js for now and we can reconsider for a further PR moving it elsewhere. Like I just want to make sure this much improved design makes it into 3.4 :)
Thank you @JoomliC for your determination on getting this right. Personally I honestly appreciate your hard work on this one!
@dgt41 it was much more a community determination, and a community improvement! ;-)
Thank you to have been an important contributor of this improved design!
This definitely shouldn't stop this from being merged, but just wanted to highlight it since I noticed it with the PR applied.
On the new Update Sites view in the Extension Manager, there's some extra whitespace just under the toolbar (the updated sidebar display makes this really evident). It'd be nice to get it in line with the rest of the views before we release stable.
In that view is missing <div class="row-fluid">
after form
A change in https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_installer/views/updatesites/tmpl/default.php#L19
from class installer-manage
to row-fluid
will fix that!
I would rather add the div below the form
otherwise we may have a missing closing div
Sorry my previous comment will not solve the prob. But deleting line 19 and line 125 will (removing the <div id="installer-manage”>
and the closing </div>
)
It does. Will merge now as soon as Travis is happy.
(But I do not have IE8 and Windows)
It does. Will merge now as soon as Travis is happy.
I'm on mac, and not able to correctly test it on IE8... so, would be welcomed to be sure it is IE8 compatible ?
Yes we need a windows test
I am going to test on IE8 + Win 7, just give me a bit of time :)
@JoomliC unfortunately I have bad news regarding IE8 on Win 7. See attached screenshot:
Is there anything I can do to help you figure this out?
Btw, I downloaded the virtual machine from http://modern.ie if that is of any help to you.
@dgt41 That code is already there at line 79.
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5141.
@JoomliC Can you do something like
if (window.outerWidth) {
var main_height = jQuery('#j-main-container').outerHeight()+30;
var sidebar_height = jQuery('#j-sidebar-container').outerHeight();
var body_width = jQuery('body').outerWidth();
var sidebar_width = $sidebar.outerWidth();
var content_width = jQuery('#content').outerWidth();
} else {
var main_height = jQuery('#j-main-container').height()+30;
var sidebar_height = jQuery('#j-sidebar-container').height();
var body_width = jQuery('body').width();
var sidebar_width = $sidebar.width();
var content_width = jQuery('#content').width();
}
@dgt41 Ok, I replaced the lines 229 - 234 with the code above in the core-uncompressed.js and enabled debug mode to be sure it is used. We are getting somewhere but not there yet:
The X is not in the right place and notice the JS error in regards to this line: var main_height = jQuery('#j-main-container').Height()+30;
Try replacing var main_height = jQuery('#j-main-container').Height()+30;
with:
var main_height = jQuery('#j-main-container’).height()+30;
height lowercase
This should be your code:
if (window.outerWidth) {
var main_height = jQuery('#j-main-container').outerHeight()+30;
var sidebar_height = jQuery('#j-sidebar-container').outerHeight();
var body_width = jQuery('body').outerWidth();
var sidebar_width = $sidebar.outerWidth();
var content_width = jQuery('#content').outerWidth();
} else {
var main_height = jQuery('#j-main-container').height()+30;
var sidebar_height = jQuery('#j-sidebar-container').height();
var body_width = jQuery('body').width();
var sidebar_width = $sidebar.width();
var content_width = jQuery('#content').width();
}
I wonder if an issue on IE8 with position:absolute and left in percentage ? (negative left position)
My bet is %
@dgt41 @roland-d Good news i hope, the issue is not in script, nor in css styling.
It is only because IE8 does not support media queries!
So, @roland-d, if you can test the css without the @media (min-width: 768px) {
And tomorrow (midnight now in France) i will update the css file to be able to work both on all platforms. ;-)
I have a VM with IE8 here, I ll give it a go!
@JoomliC Confirmed I used the script above and removed the media queries
Video here: https://dl.dropboxusercontent.com/u/55769984/ScreenFlow.mp4
Labels |
Removed:
?
|
Labels |
Added:
?
|
Took off the RTC label for now.
Can you push that as well before I give it a go?
It is updated!
I've tested on ie8/xp, safari (macbook/iphone 4S), chrome (mac), firefox (mac)
I can confirm that it works with my extension and on IE11.
However I'm not really convinced that it's an improvement. But I'm no UX guy, so bear with me. Imho the existing sidebar in staging looks better.
Reasons:
It may be a very subjective opinion from myself. But before we are going to merge this, I would love to get input from some designer/accessibility people.
@phproberto You want to point your group to this PR and get their feedback?
@test @infograf768
Tested with IEtester on Windows7 with IE8: Works like a charm
Hi all :)
Sorry to bring a raincloud to the party but, I'm sorry to say, I have very serious concerns with this PR, and even greater concern with the introduction of a collapsible sidebar in general.
There are very serious UX and accessibility issues already in the admin interface and, IMO, a collapsible sidebar can only add to those problems.
I have to ask the obvious here, why would anyone want to hide the sidebar?
My $0.02:
Doesn't hiding the side bar achieve one of the things we wanted to do in
the ux sprint last year - increasing the width of the manager screen and
removing the sub menus? Admittedly in a compromise method
On 27 Nov 2014 09:04, "Seth Warburton" notifications@github.com wrote:
Hi all :)
Sorry to bring a raincloud to the party but, I'm sorry to say, I have very
serious concerns with this PR, and even greater concern with the
introduction of a collapsible sidebar in general.There are very serious UX and accessibility issues already in the admin
interface and, IMO, a collapsible sidebar can only add to those problems.I have to ask the obvious here, why would anyone want to hide the
sidebar?My $0.02:
- If the sidebar contains anything essential, don't allow it to be hidden; that can only hinder users and reduce accessibility.
- If it doesn't contain essential items, remove it. Non-essential functionality hinders users and reduces accessibility.
—
Reply to this email directly or view it on GitHub
#5141 (comment).
Rip Van Wrinkle....
The original sidebar collapse was merged quite some time ago...
The reasoning behind it was that it would free screen real-estate.
(not saying it's worse or not for UX)
Some managers also use filters in the sidebar. Was also considered that sidebar is much more user friendly than the menu dropdowns
With all respect but this issue is ongoing for over 4 month's or so and the original sidebar collapse was merged a few month's ago as well. So I do not see any ground to restart the discussion all over.
For me the thing is RTC
Doesn't hiding the side bar achieve one of the things we wanted to do in
the ux sprint last year - increasing the width of the manager screen and
removing the sub menus? Admittedly in a compromise method
No, I wanted to see the UI simplified by removing the sidebar. This PR complicates the UI further and, IMO, brings no benefit.
Some managers also use filters in the sidebar.
Yes, another example of the complicated UI and poor UX. Is it a space for menu items or filters? Or both? Who knows, it changes in every view. In some views filters are at the top of the main content area…
Certainly the poor use can never be sure, or be able to easily comprehend the interface. A user-friendly UI is a consistent UI.
On 27 Nov 2014 09:19, "Seth Warburton" notifications@github.com wrote:
We already have hidden child menu items, in the primary navigation:
No the point we tried and failed to achieve at the UX sprint was to remove
them from the sidebar. This pr achieves that in a compromise method.
But as JM says the sidebar has already been added the only question
remaining is does it need to be visually improved.
If the sidebar contains anything essential, don't allow it to be hidden; that can only hinder users and reduce accessibility.
The sidebar can contain something essential but not needed all the time, for example the filters on some screens. As for accessibility, Hathor is there for accessibility.
No, I wanted to see the UI simplified by removing the sidebar.
That would be another PR for an altered design.
But as JM says the sidebar has already been added
A decision I'm absolutely stunned by.
Hiding something behind two-clicks, where previously it required one, is bad for accessibility and bad UX.
Add to that that it doesn't improve the UI at all, adds another JS dependency, and doesn't even use Bootstrap's methods for toggling visible items, I find it unbelievable that this was merged.
As for accessibility, Hathor is there for accessibility.
Are you serious?? Because accessibility is only about blind people, right?
Why would 'normal' people want stuff to be accessible??
Are you serious?? Because accessibility is only about blind people, right?
Why would 'normal' people want stuff to be accessible??
I am very serious, why wouldn't I be? I understood you meant accessibility as for people with visibility problems.
As for accessibility, Hathor is there for accessibility.
I disagree with that. Having Hathor still around (do people even still use it?) is no excuse for decreasing accessibility in Isis. The goal should be to raise accessibility in Isis to a point where we can drop Hathor.
Hiding something behind two-clicks, where previously it required one, is bad for accessibility and bad UX.
I don't see a problem with the (already merged) hiding feature itself. It's visible by default and can be hidden if someone wants to have more real estate. So by default nothing is hidden and everything is as in 3.3.6.
The goal should be to raise accessibility in Isis to a point where we can drop Hathor.
That I do agree with but shouldn't it be the goal of a PR/Working Group altogether? To have a complete approach rather than bits and pieces all over the place.
I don't see a problem with the (already merged) hiding feature itself. It's visible by default and can be hidden if someone wants to have more real estate.
It isn't accessible, either from a user perspective (it isn't keyboard accessible) or from the code standpoint (toggling the display property with JS!?!).
That I do agree with but shouldn't it be the goal of a PR/Working Group altogether? To have a complete approach rather than bits and pieces all over the place.
I think a great start-point would be to not make things worse than they already are…
Tabs are not accessible either via keyboard as far as I know. As a lot of stuff in Isis.
This PR is not meant to be a long term solution.
The best future proposition would be a new designed template for admin, which project is in the roadmap.
Why a sidebar ?
Why we can't remove the sidebar ?
Why hidding option for the sidebar ?
Why a sliding effect ?
Why to keep open/close in browser localstorage ?
Why js for toggling ?
Good reading ?
I don't see the show/hide sidebar as a wrong approach (when using adobe software, i understand the goal of open/hide arrows for the tools/info boxes, and the sidebar is a "tools" box). The purpose of this PR is to add an easy understandability of this behaviour.
In all cases, we can't remove today the sidebar, because of B/C issues for third party extensions (and this would need to add searchtools to all core components of Joomla).
The goal is a more flexible, and easier admin template. This could not be fully achieved yet, but if we can add some options for this, let's go!
And Hide/Show sidebar is one of those options.
With all respects...
We already have the sidebar in core at the moment in the 3.4 alpha. What I suggest is that we merge this PR for now otherwise we have the worst of both worlds. We get the sidebar (which Seth doesn't like from the UX standpoint) and we don't get the improved styling (which many of us want) while we sit here debating it.
Then we can talk over a reversion PR for the entire sidebar stuff separately if that is the way Seth feels is needed from the UX team's perspective.
+1 for me and sounds like a good plan. We need J3.4 out asap!
On 11/27/2014 6:32 PM, George Wilson wrote:
We already have the sidebar in core at the moment in the 3.4 alpha.
What I suggest is that we merge this PR for now otherwise we have the
worst of both worlds. We get the sidebar (which Seth doesn't like from
the UX standpoint) and we don't get the improved styling (which many
of us want) while we sit here debating it.Then we can talk over a reversion PR for the entire sidebar stuff
separately if that is the way Seth feels is needed from the UX team's
perspective.—
Reply to this email directly or view it on GitHub
#5141 (comment).
We need J3.4 out asap!
This PR is not a release blocker. This PR is only about styling things, which is a minor issue at best. So 3.4 could be released without this PR being merged.
One short answer to the second point of Thomas’s first comment
The buttons are not colored blue, but with the same color as links. The idea behind this is that these two links are:
1. Clickable
2. Buttons with plain/raw output
For both cases following the color of the rest of the links in the page is a good point, rather than introducing a brand new grey (or whatever color) styling that doesn’t really follow any already known styling rules.
My point is that, as they are currently styled, they are in line with the btn btn-link
of Bootstrap 2.3.2
I hope this makes clear why they are colored like that...
This PR is not a release blocker. This PR is only about styling things, which is a minor issue at best. So 3.4 could be released without this PR being merged.
@Bakual In all cases, if this PR is not merged, issues should be solved in the current sidebar of 3.4.0-alpha, as already stated by @dbhurley :
@dgt41 Thanks for this well documented information
First off, apologies all for taking this thread off at a tangent, the new to me collapsible sidebar behaviour blindsided me. If anyone else believes there's merit I'll create a new issue.
Within the scope of this PR, I still believe there is room for some easy wins.
Why a sliding effect ?
A user first see the sidebar opened. The sliding effect helps user to understand where you can open it again. The search tools button (not yet integrated for filters in all views) has already a sliding effect (it helps to see what happens!).
From my understanding this change animates opacity and width of the sidebar element directly with jQuery, which can impact performance.
I think it would be better to simply toggle a css hook on the element and perform any repositioning or opacity of the sidebar purely with css. The device requirements are lowered, more devices are supported, and this sort of functionality already exists in the underlying BS2.3.2 with the Collapse plugin.
Why js for toggling ?
IE8 compatibility.
I believe that Bootstrap are using these methods largely for device compatibility and performance reasons.
I don't see the show/hide sidebar as a wrong approach (when using adobe software, i understand the goal of open/hide arrows for the tools/info boxes, and the sidebar is a "tools" box). The purpose of this PR is to add an easy understandability of this behaviour.
I think it is the content that makes the difference in this case, as in many areas the sidebar items are effectively 2nd or 3rd levels of the admin UI primary navigation. They are, within a given task context, the fastest way to navigate between related tasks. Good UX.
Accessibility isn't simply about catering to people with vision impairments. I like to think about accessibility as a simply making things easier to use for humans. One click is always easier than two.
I totally agree with @wilsonge, @gwsdesk and @Bakual though, this PR should not be considered a release blocker. Also agree that such a change is outside the scope of this PR.
The goal is a more flexible, and easier admin template. This could not be fully achieved yet, but if we can add some options for this, let's go!
And I'm 100% behind you in this respect. The proposed changes are certainly an improvement from the current and I'd like to say thanks for creating this issue, and sorry for dropping my sidebar unhappiness in the wrong place.
Just a remark :
i can add an option in the isis template, to enable or not the sidebar effect. If disabled, will display the sidebar as it is in 3.3.6
Please not another option. I don't think users are going to find it.
Please not another option. I don't think users are going to find it.
+1. No more options please :)
Agree +100
On 11/27/2014 10:26 PM, RolandD wrote:
Please not another option. I don't think users are going to find it.
—
Reply to this email directly or view it on GitHub
#5141 (comment).
Leo Lammerink
MD GWS - Enterprise Ltd
Skype: gwsgroup
www.gws-desk.com http://www.gws-desk.com | www.gws-host.com
http://www.gws-host.com | www.gws-deals.today http://gws-deals.today
Ok for no other option in admin template, to disable the toggle effect on sidebar! ;-)
@wilsonge quote :
Meh. Maybe just leave it in core.js for now and we can reconsider for a further PR moving it elsewhere. Like I just want to make sure this much improved design makes it into 3.4 :)
@dgt41 quote:
@Fedik @wilsonge @JoomliC I am afraid that putting the code to a template related file is not an option as it will break B/C. Let’s say somebody got their own admin template, if they don’t have the script loaded in core.js their template will be broken. Also Hathor will be broken. I guess for front end we should keep shaving core.js
I think we can move core.js code to template.js, and so keep all css stuff only in isis template.
No B/C with other admin templates, if we add this in submenu.php :
jQuery(document).ready(function($) { if (typeof Joomla.toggleSidebar !== 'undefined') { Joomla.toggleSidebar(true); } else { $('#j-toggle-sidebar-header').css('display', 'none'); $('#j-toggle-button-wrapper').css('display', 'none'); } });
I think this code also can be moved to template.js
something like:
jQuery(document).ready(function($){
if($('#j-toggle-sidebar').length) Joomla.toggleSidebar(true);
});
The issue i see in moving this to template.js, is you can't remove display of collapsible sidebar header and icon, in admin templates not using the Joomla.toggleSidebar function...
@dgt41 You mean this ? :
$script = array(); $script[] = ' jQuery(document).ready(function($){'; $script[] = ' if (typeof(Joomla.toggleSidebar) !== "undefined"){'; $script[] = ' Joomla.toggleSidebar(true);'; $script[] = ' } else {'; $script[] = ' $("#j-toggle-sidebar-header").css("display", "none");'; $script[] = ' $("#j-toggle-button-wrapper").css("display", "none");'; $script[] = ' }'; $script[] = ' });'; // Add the script to the document head. JFactory::getDocument()->addScriptDeclaration(implode("\n", $script));
@JoomliC Use this instead of setting an array and then imploding:
JFactory::getDocument()->addScriptDeclaration('
jQuery(document).ready(function($){
if (typeof(Joomla.toggleSidebar) !== "undefined"){
Joomla.toggleSidebar(true);
} else {
$("#j-toggle-sidebar-header").css("display", "none");
$("#j-toggle-button-wrapper").css("display", "none");
}
});
');
@wilsonge "Meh. Maybe just leave it in core.js for now and we can reconsider for a further PR moving it elsewhere. Like I just want to make sure this much improved design makes it into 3.4 :)"
This PR updated with Joomla.toggleSidebar function moved to the template.js file of Isis.
Now, no B/C issue with third-party admin templates. ;-)
One thing I find a bit disturbing is that if the sidebar is hidden, the content still gets an "stretch" animation when reloading a page. That doesn't happen with the current implementation. Also the "X" button/link pops only up after the page is loaded.
One thing I find a bit disturbing is that if the sidebar is hidden, the content still gets an "stretch" animation when reloading a page. That doesn't happen with the current implementation. Also the "X" button/link pops only up after the page is loaded.
Thanks and well found @Bakual !
Updated and fixed ;-)
Although it is not a release blocker, it corrects some errors from the former implementation, including some lang strings changes.
Tested it OK here.
As we are going to freeze langs for beta, would be good to get it in now.
Has somebody checked this out btw for future reference?
http://demo.joomlashine.com/joomla-extensions/jsn-poweradmin/jsn-poweradmin-overview.html
I actually like it a lot compared to ISIS
On 11/28/2014 4:54 PM, Roberto Segura wrote:
Waiting for @dbhurley https://github.com/dbhurley confirmation to merge.
here because current merged version is worse than this
—
Reply to this email directly or view it on GitHub
#5141 (comment).
Please keep on topic
On 28 November 2014 at 10:10, Leo Lammerink notifications@github.com
wrote:
Has somebody checked this out btw for future reference?
http://demo.joomlashine.com/joomla-extensions/jsn-poweradmin/jsn-poweradmin-overview.html
I actually like it a lot compared to ISISOn 11/28/2014 4:54 PM, Roberto Segura wrote:
Waiting for @dbhurley https://github.com/dbhurley confirmation to
merge.here because current merged version is worse than this
—
Reply to this email directly or view it on GitHub
#5141 (comment).—
Reply to this email directly or view it on GitHub
#5141 (comment).
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
That was on topic
On 11/28/2014 5:37 PM, Brian Teeman wrote:
Please keep on topic
On 28 November 2014 at 10:10, Leo Lammerink notifications@github.com
wrote:Has somebody checked this out btw for future reference?
http://demo.joomlashine.com/joomla-extensions/jsn-poweradmin/jsn-poweradmin-overview.html
I actually like it a lot compared to ISISOn 11/28/2014 4:54 PM, Roberto Segura wrote:
Waiting for @dbhurley https://github.com/dbhurley confirmation to
merge.here because current merged version is worse than this
—
Reply to this email directly or view it on GitHub—
Reply to this email directly or view it on GitHub
#5141 (comment).Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/—
Reply to this email directly or view it on GitHub
#5141 (comment).
Leo Lammerink
MD GWS - Enterprise Ltd
Skype: gwsgroup
www.gws-desk.com http://www.gws-desk.com | www.gws-host.com
http://www.gws-host.com | www.gws-deals.today http://gws-deals.today
@gwsdesk About B/C with third-party admin template : #5141 (comment)
;-)
@nternetinspired maybe a stupid question, but do you mean $visible or $sidebar ?
No, I meant the CSS selectors like #j-sidebar-container and #j-toggle-button-wrapper in the template LESS.
@nternetinspired #j-sidebar-container is in views, not in sidebar layout, and needed to prevent issue such as the one with com_localise for exemple.
So, i don't see how to change this id by a class. Adding a new class to this id ? Something like #j-sidebar-container.jtoggle ?
Yes, it's better to add a class to the element as a hook for css, e.g. <div id="j-toggle-button-wrapper" class="j-toggle-button-wrapper">
so that your css can target .j-toggle-button-wrapper
instead.
This keeps specificity low in the cascade. An ID selector in CSS is a trump, overruling element and class selectors, which ultimately leads to stylesheet bloat and angry front-enders :)
Not sure I get it:
You want to change all views in J! as well as 3rd party components?
Not sure I get it:
You want to change all views in J! as well as 3rd party components?
Of course, no! ;-)
After a discussion with @nternetinspired @phproberto @Bakual, i have updated the PR to use only css transition, with better performance.
Only IE8 is not css transition compatible, but it's not an issue, IE8 can do without transitions.
@dgt41 So, as we have requested before, it's not a problem if no transition on ie8 as far as we can use the sidebar without issue. ;-)
@infograf768 No changes for com_localise ;-)
@nternetinspired I've tried to use http://getbootstrap.com/2.3.2/javascript.html#collapse. But it was really a headache to make it working as expected due to where the sidebar layout is loaded. I'm afraid we could have B/C issue with third party extensions (if toggle button is loaded in submenu layout) or not sure it could be good to load the button in the template index.php or elsewhere...
In fact, this could be a future improvement, with most time to check UX, and speak about it! (i will re-install my Skype to speak about a few ideas with @phproberto too ;-) )
Yes, it's better to add a class to the element as a hook for css, e.g. div id="j-toggle-button-wrapper" class="j-toggle-button-wrapper" so that your css can target .j-toggle-button-wrapper instead.
This keeps specificity low in the cascade. An ID selector in CSS is a trump, overruling element and class selectors, which ultimately leads to stylesheet bloat and angry front-enders :) ."
Done!
Note: the only way I saw about how to have a class .j-sidebar-container for div #j-sidebar-container (view) was to add class with js, and remove span2... is it correct ?
And nice week-end to all!
Cyril
works fine here...
@test Works as expected here.
Since this is already RTC, I think we can really move this forward now.
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5141.
@JoomliC Thanks for moving transition effects to css and for using class selectors. This is really a huge improvement I think.
…In fact, this could be a future improvement, with most time to check UX, and speak about it! (i will re-install my Skype to speak about a few ideas with @phproberto too ;-) )
Yes, let's look at this for a future PR. No need to further delay this one, given that it is such a big improvement already.
I'd be happy to collaborate with you on any further improvements, if you want. My skype is seths_laptop ;)
Status | Ready to Commit | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2014-11-29 16:52:29 |
Thanks to all. Merged at last!
Happy New JYear! to All !!!
@nternetinspired @phproberto @dgt41 I've requested you as contact on Skype (it was the first time i requested someone on this app i've never really used...)
My Skype : cyril_reze
Cyril
One of the biggest things in regards to the sidebar that many are telling me is 2 things:
1: It doesn't scroll with the page when it feels like it should.
2: The small toggler to open doesn't stand out and gets lost on the page. So I am attaching a mock thanks to Kelsey Brookes
Should I open a issue?
Labels |
Removed:
?
|
Labels |
Added:
?
|
@test: Tested successfully.
tested on all browsers
tested LTR and RTL
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5141.