Skip to content

Conversation

@Josh68
Copy link
Owner

@Josh68 Josh68 commented Mar 12, 2015

angular-ui#158, based on an observed (but maybe not well understood) code change by @careywalker. Basically, just move the ng-click/ng-mouseenter bindings from the LI (choices-row) to the contained DIV (choices-row-inner). So far, I haven't seen an affect on behavior in any browser, but this is with light manual testing.

All automated tests pass with these changes in place:

gulp test
[15:05:36] Using gulpfile C:\web\sites\ui-select\gulpfile.js
[15:05:36] Starting 'clean'...
[15:05:36] Finished 'clean' after 14 ms
[15:05:36] Starting 'scripts'...
[15:05:36] Starting 'styles'...
[15:05:36] Finished 'styles' after 61 ms
[15:05:37] Finished 'scripts' after 1.01 s
[15:05:37] Starting 'build'...
[15:05:37] Finished 'build' after 14 μs
[15:05:37] Starting 'karma'...
[15:05:38] Finished 'karma' after 880 ms
[15:05:38] Starting 'test'...
[15:05:38] Finished 'test' after 5.63 μs
INFO [karma]: Karma v0.12.31 server started at http://localhost:9876/
INFO [launcher]: Starting browser Chrome
INFO [Chrome 41.0.2272 (Windows 7)]: Connected on socket gaw3nxxZW2Omee4BBX-X with id 17666700
Chrome 41.0.2272 (Windows 7): Executed 110 of 110 SUCCESS (2.483 secs / 2.465 secs)

Josh68 and others added 7 commits March 12, 2015 15:09
angular-ui#158, based on an observed (but maybe not well understood) code change by @careywalker. Basically, just move the ng-click/ng-mouseenter bindings from the LI (choices-row) to the contained DIV (choices-row-inner). So far, I haven't seen an affect on behavior in any browser, but this is with light manual testing.

All automated tests pass with these changes in place:

>gulp test
[15:05:36] Using gulpfile C:\web\sites\ui-select\gulpfile.js
[15:05:36] Starting 'clean'...
[15:05:36] Finished 'clean' after 14 ms
[15:05:36] Starting 'scripts'...
[15:05:36] Starting 'styles'...
[15:05:36] Finished 'styles' after 61 ms
[15:05:37] Finished 'scripts' after 1.01 s
[15:05:37] Starting 'build'...
[15:05:37] Finished 'build' after 14 μs
[15:05:37] Starting 'karma'...
[15:05:38] Finished 'karma' after 880 ms
[15:05:38] Starting 'test'...
[15:05:38] Finished 'test' after 5.63 μs
INFO [karma]: Karma v0.12.31 server started at http://localhost:9876/
INFO [launcher]: Starting browser Chrome
INFO [Chrome 41.0.2272 (Windows 7)]: Connected on socket gaw3nxxZW2Omee4BBX-X with id 17666700
Chrome 41.0.2272 (Windows 7): Executed 110 of 110 SUCCESS (2.483 secs / 2.465 secs)
…butes on .ui-select-choices-row-inner specifically in IE8, which cannot capture those events from the parent .ui-select-choices-row. Fixes ability to select options in IE8, at least when binding the click event. Leaves functionality as-is for modern browsers.
No need for verbose conditionals
Broke things by referencing undefined document. Add dependency on $window to reference $window.document.addEventListener.
@RonaldTreur
Copy link

Chiming in to say I've been using this fix (last version) in production and it works splendidly. I hope it gets included soon, since not being able to use Bower to install this dependency is driving me crazy ;-)

Does this need a test btw?

@Josh68
Copy link
Owner Author

Josh68 commented May 6, 2015

It passes tests (broke briefly after an update, but I fixed). Ultimately, I
forked my fork (!) and merged just this change to master of that repo. I
finally realized that Bower can reference a Github repo (and even a
specific branch), so no need to wait for the pull request to get
incorporated. Just create your own fork and do the same in your project.

On Wed, May 6, 2015 at 4:18 AM, Ronald Treur [email protected]
wrote:

Chiming in to say I've been using this fix (last version) in production
and it works splendidly. I hope it gets included soon, since not being able
to use Bower to install this dependency is driving me crazy ;-)

Does this need a test btw?


Reply to this email directly or view it on GitHub
#1 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants