Skip to content
This repository has been archived by the owner on Jan 6, 2022. It is now read-only.

tabindex issue #72

Open
vipdorian opened this issue Apr 25, 2017 · 14 comments
Open

tabindex issue #72

vipdorian opened this issue Apr 25, 2017 · 14 comments
Labels

Comments

@vipdorian
Copy link

If there more than one element with tabindex >= 0, when the sidebar is opened a second time, tabindex is set to "-1" to all alements except first one.
To reproduce the bug,

  1. Browse to https://echeung.me/ng-sidebar/
  2. Click on "Opened"
  3. In Chrome inspector, change a <closesidebar=""> FOR <closesidebar="" tabindex="1">
  4. Click on "This will close the sidebar too"
    5 If click again on "Opened", the tabindex will be at "-1" instead of 1.
@arkon arkon added the bug label Apr 25, 2017
@arkon
Copy link
Owner

arkon commented Apr 25, 2017

I actually tried setting the button to have a tabindex of 2 and the anchor with closesidebar to 1. They both seem fine after opening/closing multiple times. Did you do anything else too?

@vipdorian
Copy link
Author

@arkon
Copy link
Owner

arkon commented Apr 25, 2017

@vipdorian I don't think the video's permissions are set to public.

@vipdorian
Copy link
Author

vipdorian commented Apr 25, 2017 via email

@arkon
Copy link
Owner

arkon commented Apr 25, 2017

@vipdorian Thanks, the video works now. I can reproduce it on my end too, so I'll be looking into this in the coming days.

@arkon
Copy link
Owner

arkon commented Apr 26, 2017

This is actually pretty hard to reproduce consistently, but I did look over the code and made some small edits that should hopefully fix this. Feel free to reopen this if it's still happening in v4.2.2+.

@arkon arkon closed this as completed Apr 26, 2017
@vipdorian
Copy link
Author

vipdorian commented Apr 26, 2017

Hi,
Thanks for the quick response.
I upgraded to 4.2.2 and I can reproduce it.
You can find a live example at http://ci.prescription.dorianpharmatech.com/pharmacie

I think the problem is in sidebar.component.ts at around line 411.
If the element does not have a property tabindex, it's tabindex should be set to 0.
Try to change
if (existingTabIndex !== null) {
el.setAttribute('tabindex', existingTabIndex);
}
by
if (existingTabIndex !== null) {
el.setAttribute('tabindex', existingTabIndex);
}
else {
el.setAttribute('tabindex', '0');
}

@arkon arkon reopened this Apr 26, 2017
@arkon
Copy link
Owner

arkon commented Apr 27, 2017

I don't think explicitely setting a tabindex is correct either. After all, elements don't have it normally anyway.

@vipdorian
Copy link
Author

vipdorian commented Apr 27, 2017 via email

@arkon
Copy link
Owner

arkon commented Apr 27, 2017

That's intentionally done so that the elements aren't focusable when the sidebar is closed (and presumably out of the visible viewport). The current logic should already account for removing the tabindex if there was none initially.

@arkon arkon modified the milestone: v4 May 30, 2017
@k-schneider
Copy link

k-schneider commented Apr 3, 2018

I have a collapsible menu inside of an ng-sidebar. When the parent items are closed I set tabindex to -1 for children:

[attr.tabindex]="item.open ? 0 : -1"

The sidebar appears to completely blow away any custom tabindex I'm setting though. Is there any way to disable this behavior?

Edit: It appears to be an issue with having the sidebar opened initially when the page loads. After looking at how this lib tracks tabindex I tried adding a small delay to when the sidebar opened flag is flipped to true (based on screen size) and this seems to have fixed the issue with the initial custom tabindex being lost.

@arkon
Copy link
Owner

arkon commented Apr 3, 2018

Yeah, it's because it removes any tabindex value assuming that it was previously set as -1 when closing the sidebar. I can probably throw together a workaround tonight.

@arkon
Copy link
Owner

arkon commented Apr 3, 2018

@k-schneider That issue should be fixed in v7.1.0.

@k-schneider
Copy link

Thanks! But something funky is still happening if the user closes/opens the menu too quickly. The indexes are not being restored as expected.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants