Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dropdowns update - keyboard, touch and submenus #5067

Closed
wants to merge 1 commit into from
Closed

Dropdowns update - keyboard, touch and submenus #5067

wants to merge 1 commit into from

Conversation

blakeembrey
Copy link
Contributor

Fixes:

  • Keyboard navigation using submenus (acts similar to right-click in Mac, going up first starts to top of list, bottom the bottom - moves from there, etc. Only difference is that the submenus display on item focus)
  • Takes into account the .dropup, .dropdown and .pull-left modifier classes. E.g. If it's pull-left the left and right arrows switch around, if it's a dropup submenu - we start by focusing the last item vs the first which is at the top.
  • Touch navigation now works prevents the default click events from happening with the delay - looks ugly
  • [esc] key will bring you back in focus with the initial button no matter where in the dropdown you are (and tapping it again reopens the dropdown, like previous behaviour)

@blakeembrey
Copy link
Contributor Author

This should close #5054

@thezoggy
Copy link

should squash commits

@blakeembrey
Copy link
Contributor Author

@thezoggy Just squashed all the commits. Should be a little easier to read now

@blakeembrey
Copy link
Contributor Author

What's the opinion on trigger click events from a touchstart within the dropdown. I find it extremely ugly on iOS devices when I click a link in the dropdowns and 300ms later the click appears. Just seems sort of dysfunctional.

@zawaideh
Copy link

I am finding that I need a call to preventDefault() to get this working on Android and iOS.

$('body').on('touchstart.dropdown', '.dropdown-menu', function (e) { e.stopPropagation(); e.preventDefault(); })

@abhisec
Copy link

abhisec commented Sep 26, 2012

The dropdown inside tabs still doesn't respond to touch events:
http://twitter.github.com/bootstrap/javascript.html#tabs

if you click on the dropdown in above example on ipad, there is no event when you select @fat or @mdo

@blakeembrey
Copy link
Contributor Author

@abhisec Thanks for the report. I will fix that up later today or tomorrow for you. Shouldn't be too much of a change.

@andrewdeandrade
Copy link

Hey @blakeembrey,

Thanks for opening this pull-request! Unfortunately, it looks like it fails to pass the criteria neccessary for submitting to bootstrap. The following things are currently failing:

  • should always include a unit test if changing js files

For a full list of issue filing guidelines, please refer to the bootstrap issue filing guidelines.

thanks!

@GeoffTidey
Copy link

thanks for this @blakeembrey 👍

I've been pulling my hair out for a few hours with this....

@blakeembrey
Copy link
Contributor Author

@abhisec Are you still having an issue with the tabs against my PR? I just tested it and it appears to be working.

@abhisec
Copy link

abhisec commented Oct 1, 2012

@blakeembrey thanks will try now. is this going to be merged if all looks rosy?

@blakeembrey
Copy link
Contributor Author

@thezoggy @abhisec @GeoffTidey @zawaideh If you guys have time and are looking into fixes for touch and keyboard navigation still, could you please try my latest updates? Should contain a few fixes for touch navigation and plenty for keyboard.

@verschoof
Copy link

Is there a way to disable the keyboard navigation?
I only want to use this for the sub sub nav =)

@blakeembrey
Copy link
Contributor Author

Should be able to just use $('body').off('keydown.dropdown.data-api');. Any reason in particular?

@verschoof
Copy link

  1. extra unneeded functionality
  2. It does not work in my application

So it works half now, and I don't want to spend "extra" time to find out how to fix this extra functionality.
thats why I prefer to disable it.

@blakeembrey
Copy link
Contributor Author

@verschoof No problem. Thanks for your comments - I don't suppose you have example of the markup, etc. you are using? If you do, I can check it out (in case there is something I missed).

…into account .pull-left, .dropup, .dropdown modifier classes). Takes into account whether we entered the menu using up or down. Behaves much like a standard dropdown menu on Mac, minus the submenu showing on focus. As a result of this - .pull-left will cause navigation to the submenu to be the left arrow v right normally. Use opposite direction to go up a level. Use [esc] will bring you back to the initial button focused. Reenables and adds touch support to menus and submenu items.

Conflicts:
	js/bootstrap-dropdown.js
@mdo mdo closed this Nov 14, 2012
@blakeembrey
Copy link
Contributor Author

@markdotto Could I find out why the pull request wasn't accepted?

@coling
Copy link

coling commented Nov 22, 2012

I would like to know too. I'm trying to track down the "official" final fix for issue #5094 so I can cherry pick it...

@mdo
Copy link
Member

mdo commented Nov 30, 2012

@blakeembrey I really don't know why this was closed. Phantom closures :. Will try to investigate at work.

@Yohn
Copy link
Contributor

Yohn commented Nov 30, 2012

@mdo, I saw someone else say they all got closed out when the pull request was towards 2.1.2-wip and you or someone removed that branch

@mdo
Copy link
Member

mdo commented Nov 30, 2012

@Yohn Hmm, I never took any action to do that as far as I know. Very odd.

@Yohn
Copy link
Contributor

Yohn commented Nov 30, 2012

@mdo, maybe it was because you renamed the 2.1.2-wip branch to 2,2,2-wip? im not sure what happened to that branch, but something did cause its not there

@jwg2s
Copy link

jwg2s commented Jan 2, 2013

Does this work on the Android default browser?

EDIT: It does seem to work with default android browser.

@blakeembrey
Copy link
Contributor Author

@jwg2s Thanks for the response. I never tested on Android - but this PR is fairly old now. I have got an updated copy of the script at https://raw.github.com/blakeembrey/fundamentum/master/js/dropdown.js (isn't written to Bootstrap standards, but fully compatible). If you want to give it a go I would be grateful, and it still contains all the previous changes and fixes I made.

@pseidemann
Copy link

hello,

unfortunately sub-submenus in the navbar still don't work in v2.2.2 on iOS.
the first submenu works but when I click on the second level, the whole dropdown gets closed. so it's impossible to click links in the second submenu

@blakeembrey
Copy link
Contributor Author

@pseidemann This PR was closed and isn't likely to help with your issue. If you want - your welcome to try the file I linked to and see if it fixes your issues. On a side note, Bootstrap is removing dropdown submens in a future release.

KenYN added a commit to KenYN/huginn that referenced this pull request Jan 9, 2014
DataMinerUK pushed a commit to DataMinerUK/huginn that referenced this pull request Oct 6, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.