Skip to content
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

Support shadowDom in displayed #23

Closed

Conversation

bennypowers
Copy link

closes #22

@nathanboktae
Copy link
Owner

Thanks for the submission! One area this regresses on is CSS with !important will override inline styles. Got an idea how you can handle both scenarios?

@bennypowers
Copy link
Author

What about one-size fits all: always use window.getComputedStyle?

Sent from my LGE Nexus 5 using FastHub

@bennypowers
Copy link
Author

bennypowers commented Jun 27, 2017

BTW I'm using this on elements that have display: none ! important but no style attribute and it works. Also passes yarn test.
also appears to work with elements that have a style attr
Sent from my LGE Nexus 5 using FastHub

@nathanboktae
Copy link
Owner

The 3rd case is users testing a Dom node not attached to the body with an inline display style. getComputedStyle will always return "" for displayed, despite inline styling.

@bennypowers
Copy link
Author

Can we check for that case specifically?

if el is not attached and has style.display
  check style.display
otherwise
  check getComputedStyle

?

@bennypowers
Copy link
Author

See also this incredible library https://github.com/UseAllFive/true-visibility

@bennypowers
Copy link
Author

Take a look at this gist. I wasn't able to get it to work in all cases.
it's giving false negatives on cases like this:

https://gist.github.com/bennypowers/80cfb099afc578d6e601dbde1ac9a180

test('edit controls should display', (done) => {
  flush(() => {
    edit.$.frameNext.should.be.visible;
    done();
  });
});

Benny Powers added 2 commits July 2, 2017 11:30
@nathanboktae
Copy link
Owner

Can we check for that case specifically?

That's what the current code does...

See also this incredible library

Meh, "true visibility" is a bag of hurt and can go on forever if you want. For example some things not covered in that snippet

  1. opacity: 0.01
  2. absolutely positioned element off screen (it may be covered)
  3. absolutely position element overlapping this element
  4. font and background color are the same
    ....

Really I'd rather have something like that separate from this library. There are no tests for all those cases too. Not to mention calling getComputedStyle is expensive and it's called everywhere, etc.

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.

False negatives if element is in shadowDom
2 participants