Skip to content

do not show tab when disabled - #20174

Closed
maxbeatty wants to merge 3 commits into
twbs:v4-devfrom
maxbeatty:v4-dev-19849
Closed

maxbeatty wants to merge 3 commits into
twbs:v4-devfrom
maxbeatty:v4-dev-19849

Conversation

@maxbeatty

Copy link
Copy Markdown
Contributor

attempts to fix #19849

Comment thread js/tests/unit/tab.js Outdated

$(tabsHTML)
.find('li:last a')
.on('show.bs.tab', function (e) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected function expression prefer-arrow-callback
'e' is defined but never used no-unused-vars

@maxbeatty

Copy link
Copy Markdown
Contributor Author

Is it better to fix the @houndci-bot comments or keep with the existing style of the test file?

@cvrebert cvrebert added js JavaScript or TypeScript sources and plugins v4 v4, the frozen docs on gh-pages labels Jun 25, 2016
@XhmikosR

Copy link
Copy Markdown
Member

I guess you can ignore the hound issues.

@maxbeatty

Copy link
Copy Markdown
Contributor Author

👍 happy to update the whole file so it passes hound. whatever I can I do to move v4 forward ⏩

Comment thread js/tests/unit/tab.js

$(tabsHTML)
.find('li:last a')
.on('show.bs.tab', function () {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected function expression prefer-arrow-callback

@maxbeatty

Copy link
Copy Markdown
Contributor Author

attempted to fix @houndci-bot's comments but then tests broke because underlying phantomjs isn't new enough to handle ES2015 like const and arrow functions :(

Comment thread js/tests/unit/tab.js
/* global QUnit */
$(function () {
'use strict';
'use strict'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you remove the semicolon ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's been awhile since I did this. most likely just following eslint prompts in atom

Comment thread js/tests/visual/tab.html
</div>
</div>

<h4>Tabs with disabled tab</h4>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you add a visuel test in addition of a unit test ? Currently visuel tests are usefull to show something impossible to test in unit tests

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I felt it was a clearer way to demonstrate and verify the fix

@mdo mdo mentioned this pull request Nov 28, 2016
7 tasks done
@mdo

mdo commented Nov 28, 2016

Copy link
Copy Markdown
Member

Closing as dupe of #20795.

@mdo mdo closed this Nov 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

js JavaScript or TypeScript sources and plugins v4 v4, the frozen docs on gh-pages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants