Remove jQuery dependency (#6504) #6575
Conversation
stephaniehobson
force-pushed
the
js-u
branch
2 times, most recently
from
bd357f7
to
563da92
Compare
stephaniehobson
force-pushed
the
js-u
branch
from
563da92
to
003feb0
Compare
stephaniehobson
force-pushed
the
js-u
branch
2 times, most recently
from
9f32995
to
5cbf078
Compare
stephaniehobson
force-pushed
the
js-u
branch
from
5cbf078
to
ccc0f7f
Compare
|
Updated. |
|
This needs a rebase now that the footer PR merged. |
stephaniehobson
force-pushed
the
js-u
branch
from
ccc0f7f
to
31ed06a
Compare
|
Updated. |
|
Updated. |
| Utils.trans = function(stringId) { | ||
| return _$strings.data(stringId); | ||
| if (_strings) { | ||
| return _strings.getAttribute('data-' + stringId); |
There was a problem hiding this comment.
Hmm, this isn't quite going to work as-is.
Whilst some calls to this function use dashes to separate attribute names, other calls use camelCase, which for the jQuery API (and native dataset) is totally fine, but would break here as we're assuming dashes only.
Either this function should handle both cases (more complex), or we should choose one method over the other, and update all calls to use the same syntax. Thoughts?
There was a problem hiding this comment.
Good catch. 🎣
In a perfect world we handle both cases but in the name of expediency I'm tempted to just go with dashes and update the code base.
There was a problem hiding this comment.
OTOH there is a certain irony to talking about expediency in a 3 month old PR.
There was a problem hiding this comment.
Dash is way more common, converting to dashes.
Description
Issue / Bugzilla link
#6504
Testing