viclafouch / mui-tel-input

📌 A phone number input designed for MUI (Material ui) V6 built with libphonenumber-js
https://viclafouch.github.io/mui-tel-input
MIT License
158 stars 65 forks source link

Allow override CDN location #111

Closed boschr closed 9 months ago

boschr commented 9 months ago

Possibility to override CDN location if you want to host the assets somewhere else.

Currently we're not able to use mui-tel-input because of a (very) strict CSP header which blocks the requests to flagcdn.com.

jakeisnt commented 9 months ago

I would like this change as well. I think cdnLocation should never be optional (will break URLs otherwise without a proper null check) and have a default argument of 'flagcdn.com' in both places

boschr commented 9 months ago

I would like this change as well. I think cdnLocation should never be optional (will break URLs otherwise without a proper null check) and have a default argument of 'flagcdn.com' in both places

null is not an accepted value from the type definition, cdnLocation can be string | undefined, but not null. But still, if you want to break the flags, you can use an unexisting location or just a random string like "abc".

I don't get why you would define a default value on each location (like now is done for the flagSize). To me there is no benefit in that.

jakeisnt commented 9 months ago

I would like this change as well. I think cdnLocation should never be optional (will break URLs otherwise without a proper null check) and have a default argument of 'flagcdn.com' in both places

null is not an accepted value from the type definition, cdnLocation can be string | undefined, but not null. But still, if you want to break the flags, you can use an unexisting location or just a random string like "abc".

I don't get why you would define a default value on each location (like now is done for the flagSize). To me there is no benefit in that.

Semantics of null | undefined aside, the type definitions allow for passing the value undefined, which is incorrect

boschr commented 9 months ago

I would like this change as well. I think cdnLocation should never be optional (will break URLs otherwise without a proper null check) and have a default argument of 'flagcdn.com' in both places

null is not an accepted value from the type definition, cdnLocation can be string | undefined, but not null. But still, if you want to break the flags, you can use an unexisting location or just a random string like "abc". I don't get why you would define a default value on each location (like now is done for the flagSize). To me there is no benefit in that.

Semantics of null | undefined aside, the type definitions allow for passing the value undefined, which is incorrect

That is correct, you can fill in undefined as a value, but in the first place why would you do that? And second, it doesn't do anything, it still falls back to the default value. To me, this is exactly how optional props should work.

boschr commented 9 months ago

I changed the result of the getFlagSources to an array instead of an object, but you will see.

Further I implemented all your feedback as good as possible.

If there is a source defined with a src attribute it will fallback to a regular image. This is needed for inline SVG's (as the current implementation is doing for AC and TA) which are not meant to be in a <picture> as i understand from: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/source#srcset

boschr commented 9 months ago

Hee @viclafouch, did you see my changes? And what are your thoughts?

I highly prefer to make progress on this the upcoming days and get this feature merged and released. If you have more comments or suggestions I will try to implement them as soon as possible :)

viclafouch commented 9 months ago

Released ! Thanks a lot @boschr :)