Added support for options without a value attribute. Fixes #633
#671
Conversation
| @@ -308,7 +320,11 @@ describe('$(...)', function() { | |||
| }); | |||
| it('.val(): on multiple select should get an array of values', function() { | |||
| var val = $('select#multi').val(); | |||
| expect(val).to.have.length(2); | |||
| expect(val).to.have.eql(['2', '3']); | |||
There was a problem hiding this comment.
.to.eql would be more appropriate :)
There was a problem hiding this comment.
But I can English good =(
Heh, nice catch. I will fix it this weekend.
|
Besides the unnecessary |
twolfson
force-pushed
the
dev/fallback.select.content.sqwished
branch
from
db4eedb to
d6d1254
Compare
|
I have updated the PR with the requested changes. |
| @@ -1,10 +1,12 @@ | |||
| var _ = require('lodash'), | |||
| static = require('../static'), | |||
There was a problem hiding this comment.
static is reserved as a future keyword. Perhaps we should use static_ instead.
There was a problem hiding this comment.
Better yet, we could avoid defining the variable altogether:
- static = require('../static'),
// ...
- text = static.text,
+ text = require('./static').text,Besides side-stepping the naming problem, this makes the file a little more maintainable--it's clear that only the exported text method is used here (as opposed to "anything in the static module").
There was a problem hiding this comment.
I used this style for consistency with the file as is. I suggest that if we make that change then all other utils should have a similar format:
isTag = require('../utils').isTag,
domEach = require('../utils').domEach,
hasOwn = Object.prototype.hasOwnProperty,
camelCase = require('../utils').camelCase,
cssCase = require('../utils').cssCase,There was a problem hiding this comment.
It looks like we are using $ for static in other files. I am going to switch to that for consistency for now:
https://github.com/cheeriojs/cheerio/blob/0.18.0/lib/api/manipulation.js#L3
twolfson
force-pushed
the
dev/fallback.select.content.sqwished
branch
from
d6d1254 to
2e6d6df
Compare
|
I have updated the PR with the requested changes. |
…ished Added support for options without a `value` attribute. Fixes #633
|
@twolfson Sorry for the delay – and thanks! |
As noted in #633, we are not properly treating
option'swithout avalueattribute. This PR adds back functionality that emulates the DOM and thus jQuery behavior. In this PR:attrandval