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

Triangle enhancements break inherit as a border color #430

Closed
sgenoud opened this issue Apr 24, 2019 · 3 comments
Closed

Triangle enhancements break inherit as a border color #430

sgenoud opened this issue Apr 24, 2019 · 3 comments

Comments

@sgenoud
Copy link

sgenoud commented Apr 24, 2019

  • polished version: 3.2.0

Mixin/Helper/Shorthand you were using and how you were using it:

triangle({
    pointingDirection: "top",
    width: `6px`,
    height: `6px`,
    foregroundColor: 'inherit,
})

What you are seeing:

A black box

What you expected to see:

A colored triangle

Reproduction:

https://codepen.io/anon/pen/vMVqyM

It seems that browsers (tested on Firefox and Chrome) do not support the border color inherit in the form

border-color: transparent inherit transparent transparent;

but do if they are declared by side.

I would propose to revert back the triangle logic to make it work for inherit (and make the bundle slightly bigger).

This is very useful when creating tooltip arrows who can inherit the color of the border of the tooltip.

@bhough
Copy link
Contributor

bhough commented May 5, 2019

@sgenoud We will have to chew on this. It is a more than negligible increase to bundle size when just using triangle (it actually brings 2 more dependencies on board) and will be more bloated than the original due to the addition of more directions. It also bloats the final CSS for 99% of the use cases.

We will revisit this in the near future to see if we can come up with a roughly size-neutral way of supporting this while keeping the size down for everyone else. Our apologies for not catching this use case and breaking it with the new version.

@sgenoud
Copy link
Author

sgenoud commented May 8, 2019

I have made an attempt at fixing the issue without changing much of your code. I have done some limited testing and it works as expected. Note that it relies on the order of the keys in the object (I am not sure this is an issue for you).

@bhough
Copy link
Contributor

bhough commented Sep 20, 2020

This has been addressed in v4 which is now in beta and will be released in the coming weeks:

npm install polished@next
yarn add polished@next

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants