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

Explicitly ignore containers within navbar for E004 #255

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

zacechola
Copy link
Collaborator

Fix #215

  1. Ignore containers within .navbar
  2. Expand search for more than just direct children.

1. Ignore containers within `.navbar`
2. Expand search for more than just direct children.
src/bootlint.js Outdated
@@ -409,7 +409,9 @@ var LocationIndex = _location.LocationIndex;
}
});
addLinter("E004", function lintNestedContainers($, reporter) {
var nestedContainers = $('.container, .container-fluid').children('.container, .container-fluid');
var nestedContainers = $('.container, .container-fluid').filter(function(i, div) {
return $(this).find('.navbar').length === 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

indentation is off here. Use 4 spaces per indentation please. This should begin at column 12.

@zacechola
Copy link
Collaborator Author

Argh. Had my editor set for work indentation. Will fix when I get home.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 6e7f518 on zacechola:nested_containers into cd05e3c on twbs:master.

src/bootlint.js Outdated
@@ -409,7 +409,9 @@ var LocationIndex = _location.LocationIndex;
}
});
addLinter("E004", function lintNestedContainers($, reporter) {
var nestedContainers = $('.container, .container-fluid').children('.container, .container-fluid');
var nestedContainers = $('.container, .container-fluid').filter(function () {
return $(this).find('.navbar').length === 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can be simplified to .not(':has(.navbar)').

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

All this time I'd been fighting cheerio with a .find(':not(.navbar)') check. I'll make the change.

@cvrebert cvrebert added the bug label Mar 3, 2015
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 6eb502e on zacechola:nested_containers into cd05e3c on twbs:master.

@@ -409,7 +409,7 @@ var LocationIndex = _location.LocationIndex;
}
});
addLinter("E004", function lintNestedContainers($, reporter) {
var nestedContainers = $('.container, .container-fluid').children('.container, .container-fluid');
var nestedContainers = $('.container, .container-fluid').not(':has(.navbar)').find('.container, .container-fluid');
Copy link
Collaborator

Choose a reason for hiding this comment

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

This logic seems wrong. Consider:

<div class="container">
  <div class="navbar"></div>
  <div class="container"></div>
</div>

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

Successfully merging this pull request may close these issues.

Nested container check might be wrong
5 participants