mobify / pikabu

Off-Canvas flyout menu
http://mobify.github.io/pikabu/
MIT License
453 stars 51 forks source link

Remove `@support` #79

Closed jeffkamo closed 8 years ago

jeffkamo commented 8 years ago

Status: Ready for Review Reviewers: @ericawright @stewartyu @mikenikles @fractaltheory @jansepar @marlowpayne

Due to an issue in some browsers (i.e. Android 4.4.2, Galaxy S4 default browser), using @support breaks the :not() CSS pseudo selector. See here:

https://bugs.chromium.org/p/chromium/issues/detail?id=257695

In particular, this was an issue in our project here: https://mobify.atlassian.net/browse/CF-105

Also: Removed the Chrome specific fix from before, as I cannot reproduce that particular issue anymore.

Changes

Note: I've intentionally added a test case to the example's stylesheet (not the Pikabu's scss) that demonstrates the issue. The red text should be inheriting colors from their parent, but because of the presence of @support, the :not() selector that sets color: inherit isn't being respected.

fractaltheory commented 8 years ago

@jeffkamo do you know the circumstances under which the chrome-specific fix (i.e. #69) was a problem?

ericawright commented 8 years ago

:+1:

jeffkamo commented 8 years ago

@fractaltheory my understanding is that these were the repro steps for the Chrome issue:

  1. Open pikabu page in Chrome browser that's small enough that the page scrolls
  2. Scroll down a bit
  3. Open Pikabu
  4. You should see a rendering issue like the one Stewart shows in a screenshot from #66

However I have not been able to reproduce that issue, neither before making any changes, nor after. So... ya, not sure what's up with that.

fractaltheory commented 8 years ago

@jeffkamo Hm, okay. It looks though like width: 100%; was there before the fix for #66 was applied? Is it safe to remove that?

jeffkamo commented 8 years ago

@fractaltheory good point. I've added the width and it's comment back in.

jeffkamo commented 8 years ago

@fractaltheory also, I've added a test to the example in that demonstrates the issue, and I've added instructions to the PR description to see it.

I should have mentioned this earlier, but this issue only seems to occur on Android 4.4.2 (perhaps just the Galazy S4?)

fractaltheory commented 8 years ago

:+1: