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

Fix for #369 (adding 3 new rules) #373

Closed
wants to merge 9 commits into from
Closed

Fix for #369 (adding 3 new rules) #373

wants to merge 9 commits into from

Conversation

ChrisMBarr
Copy link
Contributor

3 new rules added based on discussions around issue #369

1️⃣ Disallow textual inputs that are missing the .form-control class

  • This includes all supported <input> types, <textarea> and <select>

2️⃣ Disallow radio buttons that are not enclosed in the proper structure

3️⃣ Disallow checkboxes that are not enclosed in the proper structure


note: right now these all have tests written for them that are passing with the nodeunit grunt task. However, i cannot figure out why every single test fails in the qunit grunt task. Each test reports that it gets a failure from rule 3️⃣ (about the checkboxes), even when that test HTML does not contain checkboxes....

I can't tell if there is something I am overlooking, or if these tests just aren't working on my computer.

@cvrebert
Copy link
Collaborator

cvrebert commented Apr 7, 2016

I can't tell if there is something I am overlooking, or if these tests just aren't working on my computer.

The tests are failing for the same reason on Travis too, so it's not just your machine.

@ChrisMBarr
Copy link
Contributor Author

weird... any ideas why this is happening? I can't figure it out. I mean this code seems pretty simple, and it's nearly identical to the rule below it for radio buttons. Why would this checkbox failure get thrown on every test but the radios one does not?

@cvrebert
Copy link
Collaborator

cvrebert commented Apr 9, 2016

Nothing obvious jumps out at me. I'd recommend opening one of the failing test pages in a web browser and tracing through its execution.

cvrebert added a commit that referenced this pull request Apr 10, 2016
@cvrebert
Copy link
Collaborator

Did some debugging. Found the cause of the failures and added a workaround in 000e4aa. Can you rebase against master ?

@ChrisMBarr
Copy link
Contributor Author

I got latest from master on my fork, and merged that into my branch. All tests now pass locally now. I'm not sure what the merge issue here is though...

@cvrebert
Copy link
Collaborator

Use rebase instead of merge next time...

src/bootlint.js Outdated
));
if (inputs.length) {
inputs.each(function (i, el) {
reporter('The `.form-control` class must appear on all textual `<input/>` elements, `<textarea>` elements, and `<select>` elements', $(el));
Copy link
Collaborator

Choose a reason for hiding this comment

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

<input> instead of <input/>

@cvrebert cvrebert added this to the v1.0.0 milestone Apr 11, 2016
src/bootlint.js Outdated
}).join(',')
));
if (inputs.length) {
inputs.each(function (i, el) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need for each here, I believe. Just pass inputs as the 2nd arg to reporter

@XhmikosR
Copy link
Member

@ChrisMBarr: please fetch and rebase this.

@ChrisMBarr
Copy link
Contributor Author

@XhmikosR ok, done

@ChrisMBarr
Copy link
Contributor Author

Here are the docs to add for these rules. I know there are pending PR's to add more rules, so these ID's might not match up right now with what's in the code... but you get the idea. Just wanted to get these in here before I forgot.


E052

Require all supported <input> types, <textarea> and <select> elements to have a .form-control class

All textual input elements must have this class so that they can receive the proper bootstrap styling.

The supported <input> types that require the .form-control class are:

color, email, number, password, search, tel, text, url, datetime, datetime-local, date, month, week, time

Wrong:

<input type="text" />
<select> ... </select>
<textarea> ... </textarea>

Right:

<input type="text" class="form-control" />
<select class="form-control"> ... </select>
<textarea class="form-control"> ... </textarea>

E053

Radio buttons must use the required bootstrap structures

Radio buttons must use .radio>label>input[type="radio"] structure, or the label.radio-inline>input[type="radio"] structure defined in the bootstrap documentation. The syntax for use in input groups and buttons are also allowed.

Wrong:

<input type="radio" name="optionsRadios" value="1">
<label>One</label>
  
<input type="radio" name="optionsRadios" value="2">
<label>Two</label>
<div class="radio">
  <label>
    <input type="radio" name="optionsRadios" value="1">
    One
  </label>
</div>

<div class="radio">
  <label>
    <input type="radio" name="optionsRadios" value="2">
    Two
  </label>
</div>

E054

Checkboxes must use the required bootstrap structures

Checkboxes must use .checkbox>label>input[type="checkbox"] structure, or the label.checkbox-inline>input[type="checkbox"] structure defined in the bootstrap documentation. The syntax for use in input groups and buttons are also allowed.

Wrong:

<input type="checkbox" value="1">
<label>One</label>
  
<input type="checkbox" value="2">
<label>Two</label>
<div class="checkbox">
  <label>
    <input type="checkbox" value="1">
    One
  </label>
</div>

<div class="checkbox">
  <label>
    <input type="checkbox" value="2">
    Two
  </label>
</div>

@ChrisMBarr ChrisMBarr closed this Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants