? ? Success

User tests: Successful: Unsuccessful:

avatar JoomliC
JoomliC
19 Nov 2014

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! :+1:

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 :

  • tests on all browsers are welcome
  • test to be done in LTR and RTL
  • tests with third party extensions are needed

Comments and tips are welcomed!

Cyril

j-toggle-sidebar-2

Votes

# of Users Experiencing Issue
1/1
Average Importance Score
5.00

avatar JoomliC JoomliC - open - 19 Nov 2014
avatar jissues-bot jissues-bot - change - 19 Nov 2014
Labels Added: ?
avatar yanivRozenman yanivRozenman - test_item - 19 Nov 2014 - Tested successfully
avatar yanivRozenman
yanivRozenman - comment - 19 Nov 2014

@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.

avatar zero-24 zero-24 - change - 19 Nov 2014
Category Templates (admin) UI/UX
avatar JoomliC
JoomliC - comment - 19 Nov 2014

Thank you @yanivRozenman
Could you revert and patch again ?
I've just changed compression of core.js for Travis to succeed. ;-)

avatar yanivRozenman
yanivRozenman - comment - 19 Nov 2014

@JoomliC
seem to be ok after repatching :)

avatar dgt41
dgt41 - comment - 19 Nov 2014

@test succesful. :+1: for the fade in effect on the icons!
UPD: Latest Safari, Chrome, FF, Opera on OSX

avatar losedk
losedk - comment - 19 Nov 2014

In my test I'm experiencing inline scroll in the sidebar:
skaermbillede 2014-11-19 kl 21 36 16

And could there perhaps be added some more padding around the dropdown's?

Great work btw :)

avatar losedk
losedk - comment - 19 Nov 2014

In the screenshot from #4450 you had a gradient background, did you remove that again?

avatar roland-d
roland-d - comment - 19 Nov 2014

@JoomliC Well done !! I have tested this on Windows + FF + Chrome + IE and works as expected. I agree with @losedk the gradient would be nice to have and some padding around the dropdowns will make them look better.

avatar roland-d roland-d - test_item - 19 Nov 2014 - Tested successfully
avatar roland-d roland-d - alter_testresult - 19 Nov 2014 - dgt41: Tested successfully
avatar JoomliC
JoomliC - comment - 19 Nov 2014

@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. ;-)

avatar losedk
losedk - comment - 19 Nov 2014

@JoomliC Cool! But the filters could still use some padding :)

avatar JoomliC
JoomliC - comment - 19 Nov 2014

@losedk Just done just now ;-)

capture d ecran 2014-11-19 a 23 17 25

avatar losedk
losedk - comment - 19 Nov 2014

@JoomliC awesome! :)

My last concern is if the expand button is obvious enough

skaermbillede 2014-11-19 kl 23 34 42

avatar spignataro
spignataro - comment - 19 Nov 2014

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.

avatar dgt41
dgt41 - comment - 19 Nov 2014

@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.

avatar JoomliC
JoomliC - comment - 20 Nov 2014

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 :
j-toggle-sidebar-3
Don't know if it could be better or not, for user experience in usability...?

avatar spignataro
spignataro - comment - 20 Nov 2014

@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.

avatar losedk
losedk - comment - 20 Nov 2014

@JoomliC like the idea! But agree with @spignataro, the blue bar should be moved to the right

avatar infograf768
infograf768 - comment - 20 Nov 2014

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

avatar wilsonge
wilsonge - comment - 20 Nov 2014

@infograf768 At the moment the collapsible sidebar has never gone into a release so we can make b/c breaking changes to it.

avatar infograf768
infograf768 - comment - 20 Nov 2014

@wilsonge
Sure. But we need to implement core-like filters in com_localise urgently to solve any possible issue there.
I had included in last com_localise versions a specific layout to cope withe the former merged patch.

avatar losedk
losedk - comment - 20 Nov 2014

is com_localise part of 3.4 @infograf768 ?

avatar wilsonge
wilsonge - comment - 20 Nov 2014

No. It's a Joomla supported component that we're building. It won't be part of the CMS

avatar losedk
losedk - comment - 20 Nov 2014

Ahh okay

avatar JoomliC
JoomliC - comment - 20 Nov 2014

@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 ?
j-toggle-sidebar-4

avatar infograf768
infograf768 - comment - 20 Nov 2014

@JoomliC

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

avatar dgt41
dgt41 - comment - 20 Nov 2014

@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; ?>
avatar infograf768
infograf768 - comment - 20 Nov 2014

@JoomliC
Evidently, to test, I have updated the layout to fit the new code.

avatar JoomliC
JoomliC - comment - 20 Nov 2014

@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 ;-)

See screen shot :
capture d ecran 2014-11-20 a 18 08 24

@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 ?

avatar infograf768
infograf768 - comment - 20 Nov 2014
avatar dgt41
dgt41 - comment - 20 Nov 2014

@JoomliC I am not proposing a new option here, all I said was that if you go for the blue button instead of having a predefined blue color you can match it with the color of the header on index.php.

avatar JoomliC
JoomliC - comment - 20 Nov 2014

@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...

avatar infograf768
infograf768 - comment - 20 Nov 2014

3.2.1 com_localise is an old stuff unusable. I will have to get it out of joomlacode soonish

avatar JoomliC
JoomliC - comment - 20 Nov 2014

@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)

avatar roland-d
roland-d - comment - 20 Nov 2014

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.

avatar JoomliC JoomliC - change - 20 Nov 2014
The description was changed
avatar JoomliC
JoomliC - comment - 20 Nov 2014

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!

avatar infograf768
infograf768 - comment - 21 Nov 2014

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.

avatar infograf768
infograf768 - comment - 21 Nov 2014

I see an issue though when the screen is reduced:
The maincontainer is pushed way down.
I get here:
<div id="j-sidebar-container" class="span2 j-sidebar-visible" style="height: 1439px;">
(not only for com_localise)
screen shot 2014-11-21 at 08 47 58

avatar infograf768
infograf768 - comment - 21 Nov 2014

Also, when debug is on, the sidebar covers the left part of the debug
screen shot 2014-11-21 at 08 45 03
ug

avatar blueforce
blueforce - comment - 21 Nov 2014

just a little cosmetic thing..
see the attached screenshot.
tested with chrome and firefox
bildschirmfoto 2014-11-21 um 08 45 16

avatar blueforce blueforce - test_item - 21 Nov 2014 - Tested successfully
avatar infograf768
infograf768 - comment - 21 Nov 2014

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)

avatar JoomliC
JoomliC - comment - 21 Nov 2014

@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...)

avatar infograf768
infograf768 - comment - 21 Nov 2014

Test by taking off

+       height: 100%;

in #j-sidebar-container

avatar blueforce
blueforce - comment - 21 Nov 2014

@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.

avatar infograf768
infograf768 - comment - 21 Nov 2014

Sorry, forget taking off the height. I has one issue: the dropdowns are cut off...
screen shot 2014-11-21 at 16 56 01

avatar JoomliC
JoomliC - comment - 21 Nov 2014

@infograf768 i've just updated, and removed the fixed position for button, and after testing with no height, the filters are not cut.
capture d ecran 2014-11-21 a 17 02 08

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! ;-)

avatar blueforce
blueforce - comment - 21 Nov 2014

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.

avatar JoomliC
JoomliC - comment - 21 Nov 2014

@blueforce Thanks for testing! Good news that's your issue is solved ;-)

avatar infograf768
infograf768 - comment - 21 Nov 2014

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.

avatar infograf768
infograf768 - comment - 21 Nov 2014

It is true though that when debug is on in such a case, we just have to close the sidebar.

avatar roland-d
roland-d - comment - 21 Nov 2014

@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.

avatar dgt41
dgt41 - comment - 21 Nov 2014

@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...

avatar JoomliC
JoomliC - comment - 21 Nov 2014

@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 ?

avatar roland-d
roland-d - comment - 21 Nov 2014

@JoomliC Moving the debug to the main body is indeed a task for another PR/day.

avatar dgt41
dgt41 - comment - 21 Nov 2014

@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>";
        }
avatar wilsonge
wilsonge - comment - 21 Nov 2014

Noooo pls no template specific stuff in plugins

avatar dgt41
dgt41 - comment - 21 Nov 2014

@wilsonge :+1: The same script can go at the bottom of index.php in isis so no dirty tricks in the plugin!

avatar JoomliC
JoomliC - comment - 21 Nov 2014

@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" ?
color_sidebar_icons
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 :+1:

avatar wilsonge
wilsonge - comment - 21 Nov 2014

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:
:+1:]


Reply to this email directly or view it on GitHub
#5141 (comment).

avatar JoomliC
JoomliC - comment - 22 Nov 2014

Thanks @wilsonge !
The more i look, and the more i think the right is more consistent.

I will update to make this PR ready for test! ;-)

avatar JoomliC
JoomliC - comment - 22 Nov 2014

Ready!

Good Tests and good night ;-)

avatar infograf768
infograf768 - comment - 22 Nov 2014

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.

avatar dgt41
dgt41 - comment - 22 Nov 2014

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>";
        }
?>
avatar roland-d
roland-d - comment - 22 Nov 2014

@dgt41 Test is OK now, I like the blue buttons and all should be good. I would say one more test and move this baby to be ready for commit.

avatar dgt41
dgt41 - comment - 22 Nov 2014

@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

avatar JoomliC
JoomliC - comment - 22 Nov 2014

@dgt41 added in index.php :+1:

            a, #j-toggle-sidebar-button {

Let's look at template.less now... will maybe need help...

avatar dgt41
dgt41 - comment - 22 Nov 2014

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

avatar JoomliC
JoomliC - comment - 22 Nov 2014

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!

avatar JoomliC
JoomliC - comment - 22 Nov 2014

@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

avatar dgt41
dgt41 - comment - 22 Nov 2014

@JoomliC On the latest commit the template-rtl.css you provide with the one I created using the generatecss and the less file also provided with the patch is different. Too many duplicates

avatar JoomliC
JoomliC - comment - 22 Nov 2014

@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.

avatar roland-d
roland-d - comment - 22 Nov 2014

@JoomliC @dgt41 The generatecss.php simply generates a regular css file and an rtl less file and then combines the two with the rtl one as the last file so it overrides whatever is set in the base css file.

avatar dgt41
dgt41 - comment - 22 Nov 2014

@roland-d exactly that’s the reason you put only the overrides in rtl.less!

avatar roland-d
roland-d - comment - 22 Nov 2014

@dgt41 Correct :)

avatar JoomliC
JoomliC - comment - 22 Nov 2014

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 ! ;-) )

avatar dgt41
dgt41 - comment - 22 Nov 2014

@test
RTL OK
LTR OK
success

Well done @JoomliC, nice job!

avatar JoomliC
JoomliC - comment - 23 Nov 2014

Thank you for your help @dgt41 ! ;-)

avatar dgt41
dgt41 - comment - 23 Nov 2014

@JoomliC Do you think you can sort the debug glitch on another PR?
You only have to submit this and style debug as it was before (or go further and revamp it!)

avatar infograf768
infograf768 - comment - 23 Nov 2014

@dgt41
concerning debug, one may have also to take into account $debug_lang.

avatar dgt41
dgt41 - comment - 23 Nov 2014
avatar JoomliC
JoomliC - comment - 23 Nov 2014

@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! :+1:

avatar brianteeman
brianteeman - comment - 23 Nov 2014

@test all good for m. thanks

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

avatar brianteeman brianteeman - test_item - 23 Nov 2014 - Tested successfully
avatar JoomliC
JoomliC - comment - 23 Nov 2014

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)

avatar roland-d
roland-d - comment - 23 Nov 2014

@test All good here too, tested on both Windows + FF + Chrome + IE and Mac + FF + Safari.

avatar roland-d roland-d - test_item - 23 Nov 2014 - Tested successfully
avatar roland-d roland-d - change - 23 Nov 2014
Status Pending Ready to Commit
avatar roland-d
roland-d - comment - 23 Nov 2014

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.

avatar infograf768
infograf768 - comment - 24 Nov 2014

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

avatar dgt41
dgt41 - comment - 24 Nov 2014

@JoomliC just to take care of the flickering JM is mentioning above look at the code here #5068

avatar infograf768
infograf768 - comment - 24 Nov 2014

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.

avatar dgt41
dgt41 - comment - 24 Nov 2014

Sorry, I thought it was the flickering, wrong alarm and also wrong PR that one that I referenced. More coffee...

avatar JoomliC
JoomliC - comment - 24 Nov 2014

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.

avatar dgt41
dgt41 - comment - 24 Nov 2014

use .025em; or similar?

avatar JoomliC
JoomliC - comment - 24 Nov 2014

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)+'%';
avatar roland-d
roland-d - comment - 24 Nov 2014

@infograf768 Good find. I can reproduce that problem here as well.

avatar jissues-bot jissues-bot - change - 24 Nov 2014
Labels Added: ?
avatar JoomliC
JoomliC - comment - 24 Nov 2014

@infograf768 @dgt41 @roland-d Updated, and ready for testing resizing...! ;-)

Thank you! :+1:

avatar roland-d
roland-d - comment - 24 Nov 2014

@JoomliC Ok, tested it and the sidebar icon comes back during resizing. Just one thing I noticed is that on smaller sizes the arrow is not aligned with the search box. Is that expected as the arrow doesn't move but the searchbox does?

avatar losedk
losedk - comment - 24 Nov 2014

Did we drop the 100% height again? I my opinion it looked better before

avatar dgt41
dgt41 - comment - 24 Nov 2014

@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.

screen shot 2014-11-24 at 11 03 52

avatar JoomliC
JoomliC - comment - 24 Nov 2014

@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?

avatar dgt41
dgt41 - comment - 24 Nov 2014

Maybe this with respond.js so IE8 is happy?
But this is a new project

avatar mbabker
mbabker - comment - 24 Nov 2014

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.

avatar dgt41
dgt41 - comment - 24 Nov 2014

@mbabker with all the respect I didn’t imply or said let's drop IE8 (although one day this will happen). My point was that supporting IE8 should not degrade the performance of the modern browsers.

avatar mbabker
mbabker - comment - 24 Nov 2014

Didn't read or infer that at all. My main point was to say just make sure that IE8 works.

avatar dgt41
dgt41 - comment - 24 Nov 2014

:+1: We are on the same page here!

avatar dgt41
dgt41 - comment - 24 Nov 2014

@JoomliC Another idea is to drop the bootstrap here altogether for the 2 containers (sidebar, main) and declare the sidebar’s width for the media query points (e.g. 200px open, 0px closed) and have the main with width 100%. No recalculations needed that way…

avatar JoomliC
JoomliC - comment - 25 Nov 2014

@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!

avatar infograf768
infograf768 - comment - 25 Nov 2014

PR solved here the issue I described above.

avatar dgt41
dgt41 - comment - 25 Nov 2014

@JoomliC Sorry for being an a$$ here and mentioning the performance

avatar Fedik
Fedik - comment - 25 Nov 2014

nice job :+1:
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

avatar dgt41
dgt41 - comment - 25 Nov 2014

@Fedik I support your idea, also requested this way back, but I guess we are a minority here...

avatar wilsonge
wilsonge - comment - 25 Nov 2014

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

avatar dgt41
dgt41 - comment - 25 Nov 2014

@wilsonge Fair enough, I guess we can have it in /administrator/templates/isis/js/template.js ?
And maybe move some more admin specific code there, as well?

avatar JoomliC
JoomliC - comment - 25 Nov 2014

@dgt41 don't be sorry, your help is really appreciated, and i agree that UX performance is important! :+1:

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 ?

avatar dgt41
dgt41 - comment - 25 Nov 2014

@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

avatar Fedik
Fedik - comment - 25 Nov 2014

toggleSidebar is new feature, that will be available starting from 3.4 ... or?
then I see no problem :wink:
I also think that should be no problem move to template.js next code:

jQuery(function()
{
  Joomla.toggleSidebar(true);
});

that currently in submenu.php

avatar wilsonge
wilsonge - comment - 25 Nov 2014

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 :)

avatar dgt41
dgt41 - comment - 25 Nov 2014

@test Success. Thank you @JoomliC for your determination on getting this right. Personally I honestly appreciate your hard work on this one!

avatar JoomliC
JoomliC - comment - 25 Nov 2014

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 :)

:+1:

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!

avatar mbabker
mbabker - comment - 25 Nov 2014

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.

screen shot 2014-11-25 at 11 39 54 am

avatar infograf768
infograf768 - comment - 25 Nov 2014

In that view is missing <div class="row-fluid"> after form

avatar dgt41
dgt41 - comment - 25 Nov 2014
avatar infograf768
infograf768 - comment - 25 Nov 2014

I would rather add the div below the form
otherwise we may have a missing closing div

avatar dgt41
dgt41 - comment - 25 Nov 2014

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>)

avatar roland-d
roland-d - comment - 25 Nov 2014

@dgt41 Could you make a PR with that fix? Thanks.

Let's hope we are almost there with this PR, thanks everybody for your cooperation on this. It is a perfect example of the community at work to make Joomla better !!

avatar JoomliC
JoomliC - comment - 25 Nov 2014

Thank you @mbabker !

Adding a class="clearfix" fixes it!

id="installer-manage" class="clearfix"

PR updated!

Note: could someone test this PR on IE8 and windows?

avatar infograf768
infograf768 - comment - 25 Nov 2014

It does. Will merge now as soon as Travis is happy.

(But I do not have IE8 and Windows)

avatar JoomliC
JoomliC - comment - 25 Nov 2014

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 ?

avatar infograf768
infograf768 - comment - 25 Nov 2014

Yes we need a windows test

avatar roland-d
roland-d - comment - 25 Nov 2014

I am going to test on IE8 + Win 7, just give me a bit of time :)

avatar roland-d
roland-d - comment - 25 Nov 2014

@JoomliC unfortunately I have bad news regarding IE8 on Win 7. See attached screenshot:
sidebar_ie8win7

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.

avatar dgt41
dgt41 - comment - 25 Nov 2014

@roland-d can you retest but adding this tag in /isis/index.php header? <meta http-equiv="X-UA-Compatible" content="IE=edge" />

avatar roland-d
roland-d - comment - 25 Nov 2014

@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.

avatar JoomliC
JoomliC - comment - 25 Nov 2014

Is there anything I can do to help you figure this out?

Thank you! ;-)
@roland-d Could you test by replacing all outerWidth by width

Seems that outerWidth is not well calculated on IE8 only... :-|

avatar dgt41
dgt41 - comment - 25 Nov 2014

@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();
    }
avatar dgt41
dgt41 - comment - 25 Nov 2014

@roland-d can you copy paste the uncompressed code to core.js and replace the lines 229-234 with the code above and see if that corrects the problem?

avatar dgt41
dgt41 - comment - 25 Nov 2014

Checking with quirksmode IE8 didn’t support outerWidth, outerHeight so the above code should work

screen shot 2014-11-25 at 8 29 10

avatar roland-d
roland-d - comment - 25 Nov 2014

@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:
sidebar_ie8win7_js_error
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;

avatar dgt41
dgt41 - comment - 25 Nov 2014

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();
    }
avatar dgt41
dgt41 - comment - 25 Nov 2014

@roland-d Can you verify that this corrects the problem?

avatar roland-d
roland-d - comment - 25 Nov 2014

@dgt41 I am working on it :) It works but then it errors out on the .Width().
var body_width = jQuery('body').Width();
That line.

avatar dgt41
dgt41 - comment - 25 Nov 2014

@roland-d in the else{} replace all the capital W with w and H with h. Sorry fast typing mistakes...

avatar roland-d
roland-d - comment - 25 Nov 2014

@dgt41 I replaced as suggested, no more JS errors but the sidebar is back on the right hand side as shown in my first image.

avatar JoomliC
JoomliC - comment - 25 Nov 2014

I wonder if an issue on IE8 with position:absolute and left in percentage ? (negative left position)

avatar dgt41
dgt41 - comment - 25 Nov 2014

My bet is %

avatar JoomliC
JoomliC - comment - 25 Nov 2014

@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. ;-)

avatar dgt41
dgt41 - comment - 25 Nov 2014

I have a VM with IE8 here, I ll give it a go!

avatar JoomliC
JoomliC - comment - 25 Nov 2014

I've installed XP and IE8 on a bootcamp session on my mac, and i was able to reproduce @roland-d issue and so to find the reason of it! (the css is almost ready, just waiting a confirmation of a test on IE8 running by a real windows) ;-)

avatar dgt41
dgt41 - comment - 25 Nov 2014

@JoomliC Confirmed I used the script above and removed the media queries
Video here: https://dl.dropboxusercontent.com/u/55769984/ScreenFlow.mp4

avatar roland-d
roland-d - comment - 26 Nov 2014

@JoomliC Sorry but I am unclear about what I need to change

So, @roland-d, if you can test the css without the @media (min-width: 768px) {

If I remove everything with that starts with this, the screen is a real mess :D

avatar JoomliC
JoomliC - comment - 26 Nov 2014

@roland-d remove just the line, and the line with close tag, but keep the css classes to test on ie8 ;-)

@dgt41 working with outerWidth on ie8

New update of this PR soon... Stay tuned!

avatar infograf768 infograf768 - change - 26 Nov 2014
Labels Removed: ?
avatar jissues-bot jissues-bot - change - 26 Nov 2014
Labels Added: ?
avatar infograf768
infograf768 - comment - 26 Nov 2014

Took off the RTC label for now.

avatar JoomliC
JoomliC - comment - 26 Nov 2014

@roland-d @dgt41 : i have updated the PR to work with ie8. If you can test it then ? ;-)

avatar roland-d
roland-d - comment - 26 Nov 2014

@JoomliC Ok I have tested it on IE8 and works as expected now :) Well done. Also FF and Chrome on Windows still works as expected.
/me wonders if we are finally at the finish line :tongue:

avatar JoomliC
JoomliC - comment - 26 Nov 2014

@roland-r almost the end line! I see just a minor improvement after a test on iphone (not an issue, but a little best behaviour not impacting on ie8)

avatar dgt41
dgt41 - comment - 26 Nov 2014

@JoomliC Can you push that as well before I give it a go?

avatar JoomliC
JoomliC - comment - 26 Nov 2014

@dgt41 i'm testing it before update, so ready in a little time ;-)

avatar JoomliC
JoomliC - comment - 26 Nov 2014

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)

avatar infograf768
infograf768 - comment - 26 Nov 2014

@test Macintosh FF => OK

avatar Bakual
Bakual - comment - 26 Nov 2014

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:

  • I don't see a reason to have a sliding effect. It only delays hiding/showing the sidebar. Currently it works instantly.
  • Personally, I find the existing icons fit better into the template. They follow the look for buttons we have in other places. I'm not aware that we use any blue round buttons/icons anywhere. Currently they are grey
  • I also prefer the "hamburger" icon over the little arrow. The "hamburger" is well knows as showing a menu

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?

avatar dgt41
dgt41 - comment - 26 Nov 2014

@test OK
OS X latest safari, chrome, opera, ff
Win 7 (virtual box) IE8
Win 8 IE10
Debug, debug language on/off

avatar gwsdesk
gwsdesk - comment - 27 Nov 2014

@test @infograf768
Tested with IEtester on Windows7 with IE8: Works like a charm

avatar nternetinspired
nternetinspired - comment - 27 Nov 2014

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:

  1. If the sidebar contains anything essential, don't allow it to be hidden; that can only hinder users and reduce accessibility.
  2. If it doesn't contain essential items, remove it. Non-essential functionality hinders users and reduces accessibility.
avatar brianteeman
brianteeman - comment - 27 Nov 2014

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:

  1. If the sidebar contains anything essential, don't allow it to be hidden; that can only hinder users and reduce accessibility.
  2. 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).

avatar infograf768
infograf768 - comment - 27 Nov 2014

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)

avatar nternetinspired
nternetinspired - comment - 27 Nov 2014

We already have hidden child menu items, in the primary navigation:

screen shot 2014-11-27 at 09 14 27

This PR would mean (duplicate) child menu items could be hidden in two different places. I don't see any UX merit to that.

avatar infograf768
infograf768 - comment - 27 Nov 2014

Some managers also use filters in the sidebar. Was also considered that sidebar is much more user friendly than the menu dropdowns

avatar gwsdesk
gwsdesk - comment - 27 Nov 2014

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

avatar nternetinspired
nternetinspired - comment - 27 Nov 2014

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.

avatar nternetinspired
nternetinspired - comment - 27 Nov 2014

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.

avatar brianteeman
brianteeman - comment - 27 Nov 2014

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.

avatar roland-d
roland-d - comment - 27 Nov 2014

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.

avatar nternetinspired
nternetinspired - comment - 27 Nov 2014

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.

avatar nternetinspired
nternetinspired - comment - 27 Nov 2014

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??

avatar roland-d
roland-d - comment - 27 Nov 2014

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.

avatar Bakual
Bakual - comment - 27 Nov 2014

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.

avatar roland-d
roland-d - comment - 27 Nov 2014

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.

avatar nternetinspired
nternetinspired - comment - 27 Nov 2014

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…

avatar infograf768
infograf768 - comment - 27 Nov 2014

Tabs are not accessible either via keyboard as far as I know. As a lot of stuff in Isis.

avatar JoomliC
JoomliC - comment - 27 Nov 2014

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 ?

  • giving a much easier access in the main content area

Why we can't remove the sidebar ?

  • Sidebar layout is loaded inside component view, not in admin template (i see this as a work to do for future admin template)

Why hidding option for the sidebar ?

  • To able full screen management

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!).

Why to keep open/close in browser localstorage ?

  • To able user to keep the sidebar closed or opened while navigating through administration. (let's user the power to decide)

Why js for toggling ?

  • IE8 compatibility.

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...

avatar wilsonge
wilsonge - comment - 27 Nov 2014

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.

avatar gwsdesk
gwsdesk - comment - 27 Nov 2014

+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).

avatar Bakual
Bakual - comment - 27 Nov 2014

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.

avatar dgt41
dgt41 - comment - 27 Nov 2014

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
screen shot 2014-11-27 at 2 11 17

I hope this makes clear why they are colored like that...

avatar JoomliC
JoomliC - comment - 27 Nov 2014

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 :

  • language file naming conventions
  • move of toggle layout to the correct location
  • fix small screen design

@dgt41 Thanks for this well documented information :+1:

avatar nternetinspired
nternetinspired - comment - 27 Nov 2014

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.

avatar JoomliC
JoomliC - comment - 27 Nov 2014

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

avatar roland-d
roland-d - comment - 27 Nov 2014

Please not another option. I don't think users are going to find it.

avatar nternetinspired
nternetinspired - comment - 27 Nov 2014

Please not another option. I don't think users are going to find it.

+1. No more options please :)

avatar gwsdesk
gwsdesk - comment - 27 Nov 2014

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

avatar JoomliC
JoomliC - comment - 27 Nov 2014

Ok for no other option in admin template, to disable the toggle effect on sidebar! ;-)

avatar JoomliC
JoomliC - comment - 27 Nov 2014

@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');
        }
    });
avatar Fedik
Fedik - comment - 27 Nov 2014

@JoomliC :+1:

I think this code also can be moved to template.js :wink:
something like:

jQuery(document).ready(function($){
 if($('#j-toggle-sidebar').length) Joomla.toggleSidebar(true);
});
avatar JoomliC
JoomliC - comment - 27 Nov 2014

I think this code also can be moved to template.js :wink:
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...

avatar dgt41
dgt41 - comment - 27 Nov 2014

@JoomliC At least instead of injecting it with <script> can you inject it with

JFactory::getDocument()->addScriptDeclaration(“
jQuery(document).ready(function($){
if($('#j-toggle-sidebar').length) Joomla.toggleSidebar(true);
});
“);
avatar JoomliC
JoomliC - comment - 27 Nov 2014

@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));
avatar dgt41
dgt41 - comment - 27 Nov 2014

@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");
        }
    });
');
avatar JoomliC
JoomliC - comment - 27 Nov 2014

@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. ;-)

avatar Bakual
Bakual - comment - 27 Nov 2014

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.

avatar JoomliC
JoomliC - comment - 27 Nov 2014

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 ! :+1:

Updated and fixed ;-)

avatar infograf768
infograf768 - comment - 28 Nov 2014

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.

avatar phproberto
phproberto - comment - 28 Nov 2014

Waiting for @dbhurley confirmation to merge.

:+1: here because current merged version is worse than this

avatar gwsdesk
gwsdesk - comment - 28 Nov 2014

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.

:+1: here because current merged version is worse than this


Reply to this email directly or view it on GitHub
#5141 (comment).

avatar brianteeman
brianteeman - comment - 28 Nov 2014

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 ISIS

On 11/28/2014 4:54 PM, Roberto Segura wrote:

Waiting for @dbhurley https://github.com/dbhurley confirmation to
merge.

:+1: 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/

avatar gwsdesk
gwsdesk - comment - 28 Nov 2014

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 ISIS

On 11/28/2014 4:54 PM, Roberto Segura wrote:

Waiting for @dbhurley https://github.com/dbhurley confirmation to
merge.

:+1: 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/


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

avatar JoomliC
JoomliC - comment - 28 Nov 2014

@gwsdesk About B/C with third-party admin template : #5141 (comment)
;-)

avatar nternetinspired
nternetinspired - comment - 28 Nov 2014

Sorry @JoomliC only just noticed that your css selector is an id. Can you use a class instead please?

It's fine to use an id as a js target, but not as a css selector. Ids as css selectors introduce unwanted specificity.

avatar JoomliC
JoomliC - comment - 28 Nov 2014

@nternetinspired maybe a stupid question, but do you mean $visible or $sidebar ?

avatar nternetinspired
nternetinspired - comment - 28 Nov 2014

No, I meant the CSS selectors like #j-sidebar-container and #j-toggle-button-wrapper in the template LESS.

avatar JoomliC
JoomliC - comment - 28 Nov 2014

@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 ?

avatar nternetinspired
nternetinspired - comment - 28 Nov 2014

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 :)

avatar infograf768
infograf768 - comment - 28 Nov 2014

Not sure I get it:
You want to change all views in J! as well as 3rd party components?

avatar JoomliC
JoomliC - comment - 28 Nov 2014

Not sure I get it:
You want to change all views in J! as well as 3rd party components?

Of course, no! ;-)

avatar JoomliC
JoomliC - comment - 29 Nov 2014

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

avatar infograf768
infograf768 - comment - 29 Nov 2014

works fine here...

avatar dgt41
dgt41 - comment - 29 Nov 2014

@test again on OS X FF chrome safari opera Success
Win IE8 (virtual box) Success

avatar roland-d
roland-d - comment - 29 Nov 2014

@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.

avatar nternetinspired
nternetinspired - comment - 29 Nov 2014

@JoomliC Thanks for moving transition effects to css and for using class selectors. This is really a huge improvement I think. :bow:

…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 ;)

avatar infograf768 infograf768 - change - 29 Nov 2014
Status Ready to Commit Closed
Closed_Date 0000-00-00 00:00:00 2014-11-29 16:52:29
avatar infograf768 infograf768 - close - 29 Nov 2014
avatar zero-24 zero-24 - close - 29 Nov 2014
avatar infograf768 infograf768 - close - 29 Nov 2014
avatar infograf768
infograf768 - comment - 29 Nov 2014

Thanks to all. Merged at last! :+1:

avatar JoomliC
JoomliC - comment - 15 Jan 2015

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

avatar spignataro
spignataro - comment - 4 Mar 2015

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

imgo

Should I open a issue?

avatar zero-24 zero-24 - change - 14 Oct 2015
Labels Removed: ?
avatar joomla-cms-bot joomla-cms-bot - change - 14 Oct 2015
Labels Added: ?

Add a Comment

Login with GitHub to post a comment