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

fix: fix borders bugs on chrome without breaking in firefox #319

Merged
merged 6 commits into from
Mar 9, 2019

Conversation

guastallaigor
Copy link
Member

@guastallaigor guastallaigor commented Mar 3, 2019

ref #316
close #317

Description
This PR is to fix the border bugs on Chrome again, without breaking the issues in Firefox.

I've tested in my Ubunto 18.04, in Chrome 72.0 64bits and Firefox 65.0.1 64 bits and they are working properly, even with the zoom in and out. Not sure if it's the best implementation...

Compatibility
N/A

Caveats
Please test in Safari before accepting it, and if you can in Android Chrome, because I can't test in either of those at this moment.

@guastallaigor guastallaigor added the bug Something isn't working label Mar 3, 2019
@guastallaigor guastallaigor self-assigned this Mar 3, 2019
@guastallaigor guastallaigor requested a review from a team March 3, 2019 23:30
Copy link
Member

@BcRikko BcRikko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sry, I'm late.
I reviewed the code, but I've not confirmed the display. 🙇

@virtuoushub
Copy link

I did a quick smoke test with fix-border-bugs-on-chrome and it looks good.

Android Chrome

androidchrome

Safari

safari

Firefox

firefox

@guastallaigor
Copy link
Member Author

guastallaigor commented Mar 6, 2019

Thank you very much @virtuoushub 👍
By any chance did you also test the browser zoom in and out?
Especially in Safari/Android Chrome.

@virtuoushub
Copy link

I currently only have Android Chrome available to me to . Zooming somewhat works. Noticing some weirdness when zoomed in "a lot" (did not figure out how to get %):

"somewhat" zoomed in (looks good)

0

zoomed way in (has weird spikey effect)

1
2
3

@guastallaigor
Copy link
Member Author

I would have to say that "weird spikey effect" it's a known bug with border-image that happens only in Chrome, that we haven't been able to fix it yet.
Maybe it's better to open another issue about it.

@BcRikko
Copy link
Member

BcRikko commented Mar 9, 2019

@virtuoushub Thanks for testing. Looks good 👍

@guastallaigor
I'd like to make the code easier to read. 👨‍🎓
What do you think of the following code? 🤔

@mixin border-image-repeat() {
  border-image-repeat: stretch;

  // for chrome
  @media all and (-webkit-min-device-pixel-ratio: 0) and (min-resolution: 0.001dpcm) {
    border-image-repeat: space;
  }

  // for firefox
  @supports (-moz-appearance: meterbar) {
    border-image-repeat: stretch;
  }
}

@mixin rounded-corners($isDark: false) {
  // ...
  border-image-slice: 3;
  border-image-width: 3;
  @include border-image-repeat();
  // ...
}

Copy link
Member

@BcRikko BcRikko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for changing. 😆

I just commented on 1 row.

scss/utilities/rounded-corners-mixin.scss Outdated Show resolved Hide resolved
Copy link
Member

@BcRikko BcRikko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 😆
LGTM 🎉

@BcRikko BcRikko merged commit b7b978b into develop Mar 9, 2019
@BcRikko BcRikko deleted the fix-border-bugs-on-chrome branch March 9, 2019 03:55
@BcRikko
Copy link
Member

BcRikko commented Mar 26, 2019

🎉 This PR is included in version 2.1.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Weird border on chrome android
3 participants