omeka-s-themes / centerrow

GNU General Public License v3.0
2 stars 14 forks source link

Test new flyout navigation feature #46

Closed kimisgold closed 3 years ago

kimisgold commented 3 years ago

I've created a branch, superfish-menu, for a new dropdown/flyout navigation feature, as requested in #25.

This branch supports 2 child levels of navigation within the top bar.

This needs to be tested on both desktop and mobile browsers.

katknow commented 3 years ago

@kimisgold I can only get it to look like this in browsers and on mobile:

Screen Shot 2021-07-27 at 10 39 55 PM

If you click the nav bar, it just takes you to that page no dropdown of any kind. Cleared cache/hard refreshed etc., still nothing. Double checked and I am on the superfish-menu branch, but if I pull I get a message saying there is no tracking information for that branch so I don't know if that might have anything to do with it.

sharonmleon commented 3 years ago

http://6floors.org/workspace/omeka-s/s/bigdata/page/welcome

I get all of my menus where they are supposed to be, but none of the links are clickable except for on the Android phone.

Desktop MacOS Firefox, Safari, Chrome. Android phone. IPad.

katknow commented 3 years ago

I have no idea why, but it looks different (and works) on Firefox on my Mac:

Screen Shot 2021-07-28 at 4 09 47 PM(1)

However, everywhere else on laptops (including Firefox on PC) it looks like this and isn't clickable (it seems to be the same as Sharon's?): image

Everything seems to work nicely across mobile browsers.

katknow commented 3 years ago

Just checked, and both versions of Firefox are up-to-date. It worked on Firefox on macOS Big Sur and not on Firefox for Windows 10.

sharonmleon commented 3 years ago

Are those links function in Firefox for Mac?

katknow commented 3 years ago

Yes, they are functional.

kimisgold commented 3 years ago

@katknow Can you double check and hard refresh on Firefox for macOS?

katknow commented 3 years ago

Okay, switched to master and back then hard refresh and now it's the same as the others.

sharonmleon commented 3 years ago

The only thing I have in my console is the following warning:

This page uses the non standard property “zoom”. Consider using calc() in the relevant property values, or using “transform” along with “transform-origin: 0 0”.

kimisgold commented 3 years ago

I've just pushed a fix for the unclickable links. It's ready to pull and test again @sharonmleon and @katknow.

sharonmleon commented 3 years ago

Victory! Fully functioning as intended across MacOS browser, iPad, and Android phone.

katknow commented 3 years ago

Works across browsers on laptop and mobile for me as well! Only thing is now on mobile homepage the sidebar is a little close to the text:

image

Don't know if that's just because the structuring of my pages, though.

kimisgold commented 3 years ago

@katknow That sidebar looks different because its changes are on a different branch. We don't have to worry about it for this issue, but thanks for looking out.

kimisgold commented 3 years ago

This is still having issues on touch devices, and I haven't had successful communication with the plugin developers on how to fix it. I think at this point I'm going to look for an alternative solution to implement.

kimisgold commented 3 years ago

I have an alternative approach to these menus that are friendlier to touch devices. It uses the Adobe Accessible Megamenu, which we currently use in the Thanks, Roy themes. This approach differs from the superfish branch in a few ways:

This should be tested on desktop and mobile browsers using the adobe-accessible-menu branch.

sharonmleon commented 3 years ago

@katknow Want to give this a spin?

katknow commented 3 years ago

@sharonmleon can do!

katknow commented 3 years ago

@kimisgold it may be user error, but when I try to checkout the branch I get:

error: pathspec 'adobe-accessible-menu' did not match any file(s) known to git

kimisgold commented 3 years ago

Hmm, it definitely exists here: https://github.com/omeka-s-themes/centerrow/tree/adobe-accessible-menu. Can you try pulling again before checking out?

katknow commented 3 years ago

It took a few pulls throughout the day, but for some reason it did eventually let me switch branches! Seems to work well across browsers and mobile devices. The full width nav-bar is interesting--I've not seen many sites with horizontally oriented sub-navigation (desktop and tablets). The navigation remains vertically oriented on mobile, but also shows the full width.

kimisgold commented 3 years ago

The full width megamenu approach is one we do in the default Omeka S theme, and I thought I'd replicate it here based off the complex navigation I've seen in our users' sites. (Not to mention it was easier than coming up with a custom solution to determine submenu positioning with this particular plugin.) This all sounds according to intended use and ready to merge.