-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Testing before merge to core #16
Comments
@aydun are there any specs on exactly what "Keyboard interaction works as expected" means? What key should engage the menu to start? And then should it be tab/shift-tab between top levels, enter to open sub menu and arrows to move between sub items? And enter to load? Or something else? |
Joomla testing:
|
Yes - there is a checklist on https://lab.civicrm.org/dev/accessibility/issues/1 |
Thanks, will retest @colemanw. |
@vingle - not sure, but on the early versions of this I found that I needed to hit tab a lot of times to get past things that weren't visible until the focus became visible. |
@aydun Ah that makes sense. this menu obscures the CMS menu which still needs to be tabbed thru in a hidden state, then it maybe needs to move the skip link otherwise this isn't really keyboard-accessible? Maybe that should be opened as a separate issue. |
@vingle Yes, makes sense to do that as a separate issue since it may not be a simple fix. Maybe it needs a 'skip to civi menu' as well as 'skip to content' |
When the menubar is positioned over the CMS menu, the latter should be completely hidden and not tabbable. If that's not the case let's open a new issue. |
@colemanw Reporting a couple of issues with WordPress here:
|
@christianwach hey thanks again for the testing. I wasn't able to replicate the responsiveness issue - see the screencast I just added to the readme. Can you check your css and see what's up? |
@colemanw My bad - it was some pesky interfering CSS from elsewhere. |
@christianwach thanks. I'd like to tick the WP boxes above based on your testing - can I tick off them all or are there any you haven't checked? |
With Shoreditch enabled - there are a bunch of design issues - specifically related to the menu. Not sure if there will be a shoreditch fix to resolve this or if the shoreditch menu will be replaced by this menu. Also - for those Drupal users that use an alternative admin toolbar - like adminimal_admin_menu - the mobile toolbars placement is different and not well placed. |
|
CMS agnostic - can we change the over-writable text in search field to adopt menu font? |
@christianwach Somehow this was fixed when I upgraded to WP5.0 - a bunch of other issues were fixed too. Odd. |
@themak1985 thanks for the testing. I've updated the master list. FYI to test the "Above content area" visit Administer -> Customize Data & Screens -> Display Preferences and scroll to the bottom to see the new setting. |
@colemanw on wordpress - i should note that when there is two rows of menus - it does not overlap with page content - but it does block the first item on the Wordpress left toolbar. I don't think this is an easy fix - and I think its fine personally. Also - personal note - Above Content Area menus are weird - but they work. :) Hamburger menu in wordpress works but has a small formatting issue at top |
@themak1985 it's not meant to block anything in the left toolbar - did you see the screencast I put in the readme? Also I haven't noticed that formatting issue (also see that same screencast). |
A quick test on Joomla 3.9.1, Civi 5.8.0, KAM from github master, and Firefox. Mostly working quite well (great work @aydun and @colemanw). Just saw two minor issues:
|
@themak1985 Do you have CiviCRM Admin Utilities active? And if so, have you updated to version 0.6? There's a much lighter touch in 0.6 with respect to KAM. Edit: to clarify, CAU only enables compatibility with the WordPress user's Admin Colour Scheme setting. |
@colemanw It would be nice if KAM could match the height of the WordPress admin menu too: i.e. 32px when > 768px viewport. |
WordPress@colemanw testing and feedback as requested - my comments in italic: General
Position: Over Wordpress Menu
Position: Below Wordpress Menu
Position: Above Content Area
Keyboard interaction screenshotsLoad a CiviCRM page, hit the What you should see is: Hit What I'd like to see (on hitting Details of how to enable this in my comment above. The stacking problem is because the WordPress admin bar has |
Yes - but my formatting issues persist - even with CiviCRM Admin Utilities disabled or the options disabled. |
@christianwach @colemanw - In WP, I also get the same mostly hidden skip box that you get when I push tab. Of course I don't see it at all when I have the menu run to two rows. To engage the CiviCRM menu - i shift+tab - which starts me at the menu location button - and runs from the end of the menu to the start screen - which is all functional, except I cannot change quick search functions. When the menu is below the WP top menu - Skip to main content works - however skip to toolbar - skips to the WP top admin menu - then to CiviCRM in left sidebar, and finally page content. Still the quickest way to CiviCRM menu - albeit the end of the menu is Shift+Tab In Drupal, There is no skip to toolbar option, I only get the skip to main content option, which isnt really much of a help in my case - as I could get to the content in just a couple extra tabs on most pages - maybe others have a lot more content at the top than I do. Still to access toolbar - I enter Shift+tab to get to the end of the toolbar. Quick search options are not selectable - but everything else works. |
@artfulrobot your screenshot makes it look like |
Ah it is working correctly, but has the responsive thing turned off (I must have found that annoying for some reason!). OK, please ignore this :-) |
On D7 - is Shift-Tab really the fastest way to get to the menu? When simply pushing tab, the option to skip to main content is there - but not the option to Open CiviCRM Menu. On Wordpress - Open CiviCRM Menu is there and works - but is there a way to cycle through the options without first cycling through CiviCRM Home, Hide Menu and Log Out. |
@colemanw - these are my results for Joomla. My comments are in italic.
Summary of my other points noted earlier, all others have been resolved:
|
@themak1985 Yeah I agree with this - but it may be a result of keyboard navigation being, um, inconsistent in SmartMenus itself. (FWIW I wish I could point to a page on their docs site that documents the keyboard interactions, but cannot for the life of me find one! Help?) A solution could be to comment out this line so that the dropdown is not opened when @colemanw My biggest frustration with SmartMenus is that |
Here's the link to the keyboard addon: http://vadikom.github.io/smartmenus/src/demo/keyboard-navigation.html |
@aydun Thank you - that was driving me crazy! |
@andrewpthompson this is actually a setting the extension adds to Administer -> Customize Data & Screens -> Display Preferences - the first option is the default and can be toggled on-the-fly to the 2nd option. But you have to visit Display Preferences to choose the "Above Content Area" option. I view it as kind of Legacy, but some Joomla folks like @lcdservices are really used to having it there so I put that in for them. |
@colemanw - that makes sense now. I've now tested all 3 positions on Joomla so these can now be ticked: Joomla!
|
@colemanw do you still need testing on these?
|
@themak1985 yes please! |
@colemanw Just tested in two environments for you. When the menu bar is below the wordpress menu that is wrapped to two lines - it does not overlap with page content - but it does a little hop - the same happens when its over the wordpress menu - so its fine. Alternatively - it would be nice if additional items would be in a "more" menu item instead of wrapping to an additional line. - see image example below. We never really had the two line option until shoreditch came along - the current soon to be replaced Civi menu would simply not show you any of the remaining items. Do people rely on the two line setup. As for the menu bar above content area - the Menu does not overlap page content even when wrapped to 2 rows and does collapse to a hamburger menu in the wordpress admin menu - but it does not adopt the wordpress menu color like it does when over or below the menu bar. See below. Sidebar - we use the wordpress admin style by @christianwach - but I just noticed - when that is disabled - the menu - regardless of lines or location - slightly covers the print icon. Maybe just slightly lower the print icon in core ;). Also note - the search icon in the quick search box is gone. Hamburger menu when civi menu replaces or is below wordpress menu bar. Also notice bottom of menu bar. |
@themak1985 Yup, that's one of the things that CAU fixes in CiviCRM-WordPress. The declaration is duplicated in both the admin theme and the menu fixes because the menu-fixing option used to be called "Prettify CiviCRM" and I didn't want to confound people's expectations when the admin theme was released and people hadn't enabled it. I agree that a fix for that should be in core. |
@colemanw Regarding these two Joomla tests, I'd be happy for them to be marked as having passed. I tested the normal menubar for proper touch operation on a tablet PC, with multiple browsers. Same for the mobile menu plus also on an Android 8 phone with both Chrome and Firefox browsers. Joomla!
|
@themak1985 next time that happens please right-click on it and inspect - try to find out what css rule is causing it. |
Ok, it happened again (when upgrading from Beta4 to stable) - menu shifted to bottom - I am not quite sure what to look for but I noticed that when the menu is at the bottom, the menu toggle button can be inspected somewhere up top. The toggle button is not visible when the menu is at the bottom. Maybe there is something there? Observations: For I am starting to wonder if maybe shoreditch is causing these issues - but why does clearing the cache resolve it. |
@laryn I've pushed up some more improvements:
|
I can confirm the junk at the bottom is gone on a disabled menu now, and it follows the Backdrop admin menu 'sticky' setting (if it is in "Replace Backdrop menu" mode; not if it's "Underneath Backdrop menu", which may be by design). The hamburger still shows up essentially the same for me. If I fiddle in the inspector I can adjust the CSS to get something like this: I suspect that I can't just fiddle though, or it will throw things off for other CMS's? |
@laryn this extension includes a js and css file specific to each CMS. So position tweaking for BD should be done in the The change I made does cause the position to follow the BD position setting whether it's replacing or underneath the BD Admin menu, strange that you don't see that taking effect - maybe clear your cache. I guess you're using a theme other than the stock one we get with buildkit because your BD Admin menu looks different than mine. For small screens my recent change positions the civi hamburger like so: |
@colemanw I think it is a subtheme of stock but I didn't think I made any changes that would affect the admin menu. Regardless -- I'll test again on a fresh install as soon as I get a chance. |
@colemanw Ah -- which version of Backdrop does buildkit pull? The latest version (1.12) makes significant changes in the admin menu. |
@laryn looks like buildkit pulls the latest 1.x but that change was fairly recent. I've upgraded my buildkit install and the menubar continues to correctly follow the BD setting for sticking to the top of the screen. Now the menu looks like this on small screens: Is that not what you're seeing? |
@colemanw That's not what I'm seeing but I'll test on a fresh install. Aside: it would be nice to make the hamburgers match. |
Once all other issues are resolved for this extension, the plan is to merge it into core.
See https://lab.civicrm.org/dev/core/issues/487
User Testing Needed
Please test what you can and leave a comment below to say what worked and what didn't.
The text was updated successfully, but these errors were encountered: