Meant to work with multiple matching elements?

[kevinsmith]

From what I can tell, this only works with the first matched element in the DOM. Am I just using it incorrectly, or are multiple matching elements not supported? If the latter, is this a planned feature?

[samatcd]

This currently only works with the first matched element. This could be changed to test all elements in the collection and return true only if ALL elements match the visibility rules.

It's possible that this might break existing implementations if they update though... I'd be keen on making this change maybe in a v2 lib.

[kevinsmith]

I was thinking more along the lines of returning true if ANY of the matching elements in the collection were visible. (That's actually what I expected the behavior would be.)

My use case is that I want to show a particular button only if none of the matched elements are visible to the user.

On Jul 2, 2015, at 4:48 PM, Sam Sehnert [email protected] wrote:

This currently only works with the first matched element. This could be changed to test all elements in the collection and return true only if ALL elements match the visibility rules.

It's possible that this might break existing implementations if they update though... I'd be keen on making this change maybe in a v2 lib.

—
Reply to this email directly or view it on GitHub.

[samatcd]

Ah, Ok — I can see the use case for both options. I guess we could use the 'partial' flag an make some rules about the element collection there..

E.g.,

$('.collection').visible( true ) // Requires ALL parts of ALL elements to be visible
$('.collection').visible( false ) // Requires ANY part of ANY element to be visible
$('.collection').visible( 50 ) // Requires 50% of ALL elements to be visible

Maybe something like that?

[kevinsmith]

Hmm, I'm not sure that's a good idea. It's coupling two potential desires into one config option, and that config already has an attached meaning for only one of those desires. How about taking a config object in the first parameter?

$('#element').visible({
    whole: true, // Requires ALL part of element(s) to be visible
    match: all, // Check against all matching elements
    direction: horizontal // Direction to check against
});

That would allow for adding options in the future without becoming an untenable series of arguments. And you wouldn't even necessarily have to wait for 2.0 to do that since it doesn't require a breaking change.

[kevinsmith]

I should note that I'm happy to code this up and submit a pull request if you like the idea.

[samatcd]

Yeah, very good call. I've got another issue: #31 which refers to the first parameter, but we can actually roll that functionality into this options parameter style.

If you want to have a go, I'm more than happy to accept pull requests.

[kevinsmith]

You know what? I'm not sure my contribution would even be needed. I started working on it and realized that it would probably be better for the front-end dev to handle it themselves, like so:

  var visible = false;

  $( '#element' ).each( function() {
    if ( $( this ).visible( true, true ) ) {
      visible = true;
    }

    if ( visible ) {
      // Do things if the element is visible.
    } else {
      // Do other things if it's not.
    }
  });

That's how I'm handling it on my project, and it provides the leeway to handle the logic however I want. Just thought I'd share in case anyone stumbles upon this.

[samatcd]

I'm closing this one as it's been observed that it's trivially handled outside the plugin when needed.