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

[FIXES] Fix deprecated sass, Polish translation, types, build command #1929

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

Bartek20
Copy link

@Bartek20 Bartek20 commented Jan 11, 2025

  • Fix part of vulnerabilities
  • Fix deprecated sass global functions
  • Moved from grunt-sass to grunt-contrib-sass
  • Added handling special cases in translation
  • Added injecting iti instance to input html element
  • Fix error during npm run build types generation
  • Fix generated types bug (types export utils as utils-compiled not as utils)

@Bartek20 Bartek20 changed the title Fix some problems with sass Fix deprecated sass, fix Polish translation Jan 13, 2025
@Bartek20 Bartek20 changed the title Fix deprecated sass, fix Polish translation [FIXES] Fix deprecated sass, Polish translation, types, build command Jan 16, 2025
@Bartek20
Copy link
Author

Bartek20 commented Feb 4, 2025

Hi @jackocnr,
Sorry for pinging you. When can we expect to have it merged?

I see that you released new version since this request was created but you haven't included it in release.
Can I ask in which version will it be included.

@jackocnr
Copy link
Owner

Hi there, thanks for putting this together. Unfortunately, I have very limited time to commit to open-source over the next month or so, so all I can do is merge very simple straightforward PRs e.g. basically just simple translations and LPN updates. I see you're including a translation change here, but the logic looks complicated so I'm afraid I won't be able to review this for a while.

One thing I can say is that you have included a lot of different changes in this PR, and when I have time to review it, probably in late March to be honest, I will need more information on why each change needs to happen, and why your approach fixes the issue e.g. for the Polish language change, can you describe what the current problem is, with an example, and provide a source to justify your approach here. Thanks. And apologies for the delay, but life gets in the way of open-source sometimes!

@Bartek20
Copy link
Author

Bartek20 commented Feb 18, 2025

@jackocnr np. I just wanted to know when merge can be expected.

Also here's why this changes need to happen:

  1. Fix vulneratilities - obvious
  2. Deprecated sass - deprecated function may soon be removed - @import, map (global module)
  3. Move grunt-contrib-sass - Please ignore already reverted back as fix was released on grunt-sass repo.
  4. Some languages have different form for same words in different amount of something and it can't be handled using current zero/one/more approach in counting search results
    eg. Polish: 2 wyniki but 12 wyników and 32 wyniki
    I solved it using new Function() constructor which may be considered unsafe, but input of this contructor comes directly from translations and is checked between releases so I assumed it's input is controled so should be safe.
    Alternative approach would be to add new key to translation file (something like searchResultFunction) and here implement function instead of using new Function() constructor.
  5. Inject Iti instance - Adds ability to access instance directly from DOM element (eg. document.getElementById('phoneInput').iti)
  6. Build error - Fixed error described in here and mentioned here feat: add angular component #1941.
  7. Utilities - Rn utilities are exported as intl-tel-input/utils but types file exports them as intl-tel-input/utils-compiled as it takes file name as export. I renamed file and fixed this issue (reported in TypeScript module typing for intl-tel-input/utils missing #1940)

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.

2 participants