Skip to content

bug: unreachable code in ionic.activator.start. ion-item's without ng-click cant get activated class #3170

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
djett41 opened this issue Feb 24, 2015 · 7 comments
Assignees
Labels
needs: reply the issue needs a response from the user

Comments

@djett41
Copy link

djett41 commented Feb 24, 2015

Type: bug

Platform: all

ion-item directives without ng-click, or other HTML elements with an item class that are NOT anchors or buttons will not receive the activated class on mouse/tap down/move events.

In the ionic.activator.start method line 3100 in beta14, if any condition is met that set's the eleToActivate variable, the for loop is broken out of, therefore even if your element or element's parent contains the .item class, the code below will never evaluate to true (eleToActivate will always be undefined)

if (eleToActivate && ele.classList.contains('item')) {
  eleToActivate = ele;
  break;
}

Removing the check for eleToActivate should resolve the issue

@djett41 djett41 changed the title unreachable code in ionic.activator.start. Doesn't allow for activated class for some .item elements bug: unreachable code in ionic.activator.start. Doesn't allow for activated class for some .item elements Feb 24, 2015
@djett41 djett41 changed the title bug: unreachable code in ionic.activator.start. Doesn't allow for activated class for some .item elements bug: unreachable code in ionic.activator.start. ion-item's without ng-click cant get activated class Feb 24, 2015
@mhartington
Copy link
Contributor

So if they're not elements that you can tap, then why provide any visual feed back?
I don't really see any point to this to be honest. From a usability standpoint, it would create confusion as to whats a button and what not.

@djett41
Copy link
Author

djett41 commented Feb 25, 2015

@mhartington maybe I was unclear. These are definitely tappable elements

  1. Suppose you had a ion-item or a directive with the item class, and you want it to get the activated class, but wanted to implement an touch event handler other than ng-click, or say you just used jquery lite to implement the click handler in the directive..... The element would never get the activated class.
  2. The line of code i'm referring actually includes the check (looks for .item class) which would allow Add support for "tap" virtual event #1 to get the activated class. The problem is that the first part of the condition (check is eleToActivate is defined) will NEVER BE TRUE therefore the code for the condition is unreachable. So even if you decided that you don't want to give user's the flexibility and want to restrict them to using ng-click for items... you still have unreachable code in your codebase.

@djett41
Copy link
Author

djett41 commented Feb 25, 2015

@mhartington the following issue is related to exactly what I am saying. #3050

Removing the check for eleToActivate in line 3100 in beta14 ionic.js, would allow @sgon00 to be able to use ion-item with href and still get the activated class, rather than being forced to use ng-click as he described in his example.

@mhartington mhartington reopened this Feb 25, 2015
@mhartington
Copy link
Contributor

Will pass this to @adamdbradley

@PerfectionVR
Copy link

This is in ionic.bundle.js 1.2.0 at line 3222

      var eleToActivate;
      //Two lines omitted
      if (eleToActivate && ele.classList && ele.classList.contains('item')) {
        eleToActivate = ele;
        break;
      }

If this helps any.

@jgw96
Copy link
Contributor

jgw96 commented May 31, 2016

Hello all! Is this still an issue with the latest version of ionic? Thanks!

@jgw96 jgw96 added the needs: reply the issue needs a response from the user label May 31, 2016
@jgw96
Copy link
Contributor

jgw96 commented Jun 9, 2016

Hello all! As it seems it has been a while since there was any activity on this issue i will be closing it for now. Feel free to comment if you are still running into this issue. Thanks for using Ionic!

@jgw96 jgw96 closed this as completed Jun 9, 2016
@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Sep 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs: reply the issue needs a response from the user
Projects
None yet
Development

No branches or pull requests

7 participants