Allows both OL and UL lists for tab buttons

[tiesont]

Modifies tab.js to allow OL as a parent element for the tab list items.

Duplicates UL-based visual tests to provide OL-based versions.

Adds check for "show" class to tabs unit tests.

[Johann-S]

Some of your changes are irrelevant

[XhmikosR]

@tiesont: can you take care of @Johann-S's comment?

[XhmikosR]

Closing due to lack of response. You can comment here and we can re-open it if you decide to resume working on the requested changes.

[tiesont]

I can fix this, I suppose, but I'm not sure what exactly is the issue here - I added those tests to make sure the plugin actually works with both UL and OL markup, which was the whole point of the original issue. Should I assume what you want is just the

assert.strictEqual($('#qunit-fixture').find('.show').attr('id'), 'home')

test? Or, do you only want the tests in tab.js?

[Johann-S]

That's right, you just have to create one new test, with <ol> list, which show it works 👍

[tiesont]

One unit test (which should already be there) or one visual test (which is also already there)? Is the issue that I modified the existing visual test to indicate which was UL-based and which was OL-based?

[Johann-S]

@tiesont the reason is, you have unrelated changes.
Your fix should be:

And that's all because ordered list works, but with your fix we use the correct selector, so the use case is already covered

[tiesont]

Okay, I'll admit that I'm not all that great at using Git (or GitHub, for that matter). I pulled down the 4.1.3 source, merged the updated tab.js file, and then made the updates to allow OL as a selector. I reverted the changes I made to the unit test and visual test files to match what is in this branch.

[Johann-S]

I updated your branch and removed unrelated changes.

For me we should add one thing, it's a little note on our documentation to show we accept <ol> too 🤔
You can edit that file to do that: https://github.com/twbs/bootstrap/blob/v4-dev/site/docs/4.1/components/navs.md#base-nav

[Johann-S]

@XhmikosR do you know why Github tells me there are conflicts here ? I resolved all the conflicts 😟

[XhmikosR]

I don't see any conflicts here.

[XhmikosR]

Ah, I see it now. It's shown if you switch Rebase and merge. Because there's a merge commit.

[Johann-S]

Thank you @tiesont 👍

[tiesont]

@Johann-S No problem. Thank you for being patient with me.