[pagination] Make isActive a method

[charkins]

Computed properties cannot take arguments. As-is, pagination
component is entirely broken and will not render.

[tmorehouse]

Oops, that was my mistake.

[charkins]

No problem.

Just a heads up that I'm looking at the keyboard focus stuff in paginator now. If you happen to be actively working on it, let me know and I'll hold off, otherwise I'll try to get another PR up later to fix it up. The first issue is the ref="buttons" on the multiple button elements results in this.$refs.buttons pointing only to the last button, so the find() and indexOf() functions fail in the focusXXX() methods (vue will use an array for the ref if it is only used within v-for, but subsequent ref="buttons" overrides it). I've worked around that, but now catching edge cases in focus traversal that don't seem to be handled.

[tmorehouse]

Let me play with it a bit more, and see if my logic is a bit off.

[tmorehouse]

@charkins , take a look at https://github.com/tmorehouse/bootstrap-vue/blob/tmorehouse-pagination/lib/components/pagination.vue

I have refactored how the buttons are gathered into an array

[charkins]

That's pretty much the same approach I took. Two problems:

1. focusNext() you have buttons().indexOf(...), should be buttons.indexOf(...)
2. focusPrev() you're still using this.$refs.buttons for all but the idx

With those changes the errors go away, but focus doesn't change for me. I had similar issues elsewhere in my code where I need to use this.$nextTick(()=>someElement.focus()) to get focus to actually change. When I added that approach to the paginator, focusCurrent would get called before focus changed and mess things up for me. I'll keep playing with it a bit.

What browser are you using? I've tested both chrome and firefox on linux and cannot get focus to change without using nextTick:

google-chrome-stable-58.0.3029.81-1.x86_64
firefox-52.0.2-2.fc25.x86_64

[tmorehouse]

I'm using chrome on an old OS :(

I'll make the changes you suggested and add in next tick for focusing.

[charkins]

It still doesn't quite work with nextTick. I didn't thoroughly debug yet, but I believe focusNext does nextTick focus, then focusCurrent gets called before the nextTick executes and ends up scheduling another nextTick focus which sets the focus back to the originally focused button.

[tmorehouse]

Check out this update:
https://github.com/tmorehouse/bootstrap-vue/blob/tmorehouse-pagination/lib/components/pagination.vue

[tmorehouse]

I wonder if the focusFirst() call in focusCurrent() should be wrapped also in a nextTick., which in turn wraps itself (focusFirst() in a nextTick when focusing the first, which would double nextTick it?

[tmorehouse]

Ah, I see the focus current issue now. The focusin on the pagination element needed a .self modifier (it was inadvertently receiving the focusin events from the buttons too and causing focus to go back to the current page button) The current page button should only be focused if the pagination container was tabbed into.

[charkins]

That does it! Seems obvious now.

[tmorehouse]

Yeah. one of those silly things that you can stare at for awhile and not see it.

[tmorehouse]

I'll issue a PR for the changes.

I'l;l also double check the toolbar optional keyboard navigation to make sure it is doing a .self where needed.

[charkins]

Thanks. I'll re-test once I see the changes merged upstream.

[tmorehouse]

The b-button-toolbar wrapper does have the .self on focusin, so it should be good.