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

Dutch translations and implement Rollup #8

Merged
merged 8 commits into from Aug 3, 2019

Conversation

@Favna
Copy link
Contributor

commented Aug 2, 2019

Dutch translations needs no further explanation but for Rollup lets go into some detail

RollupJS is a .. rolling up... library. Sorry, pun entirely intended. But really let's get serious. Rollup has various advantages but to list a few major ones:

  • Small bundle sizes for both Node and Browser
    • By example, I implemented it in another library just yesterday which went from ~135Kb to ~76Kb as a browser bundle (according to bundlephobia.com)
  • Treeshakeable, this is primarily awesome for browsers because there is
    no reason to load in languages that are not being used. Instead by the
    power of treeshaking only 1 language gets bundled by webpack.
  • Single tsconfig file that defines how the lib is coded, not how its
    build
  • Automatically running of Terser for minification and compression
  • If you need any convincing that it's a good thing to use, it's also
    being used by the main React library and maaaany other packages.
  • Exported the drawLocales function as both a default export and a named
    export

Lastly some minor fixes and additions that I did:

  • Found a missing / in the closing <strong> tag in the french translations. I've added it.
  • Fixed a missing capital letter in the English translations
  • Added an .editorconfig to standardise the "format document" of editors
    such as VSCode
  • Added .vscode/, .idea/ and .vs/ to .gitignore, these are folders used by IDE's to store their workspace-specific settings

Favna added some commits Aug 2, 2019

Dutch Translations
Also fixed a capital letter for the English ones

Signed-off-by: Jeroen Claassens <support@favware.tech>
Implement Rollup and minor fixes
Minor fixes and additions
- Found a missing / in the <strong> tag in the french translations. I've
added it.
- Fixed a missing capital letter in the English translations
- Added an editorconfig to standardize the "format document" of editors
such as VSCode

Rollup has various advantages but to list a few
- Small bundle sizes for both Node and Browser
- Treeshakeable, this is primarily awesome for browsers because there is
no reason to load in languages that are not being used. Instead by the
power of treeshaking only 1 language gets bundled by webpack.
- Single tsconfig file that defines how the lib is coded, not how its
build
- Automatically running of Terser for minification and compression
- If you need any convincing that it's a good thing to use, it's also
being used by the main React library and maaaany other packages.
- Exported the drawLocales function as both a default export and a named
export

Signed-off-by: Jeroen Claassens <support@favware.tech>
Finetune some Dutch texts
Signed-off-by: Jeroen Claassens <support@favware.tech>
@DenisCarriere

This comment has been minimized.

Copy link
Owner

commented Aug 3, 2019

Thanks @Favna for the extensive PR

I am familiar with Rollup and I've used it in the past

I was attempting to stick my browser build by only using Typescript, but unfortunately ES modules + Typescript isn't all that great.

Glad to merge this and publish a new minor release 👍

@DenisCarriere DenisCarriere merged commit 4b961ac into DenisCarriere:master Aug 3, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Favna Favna deleted the favware:nl-trans-and-rollup branch Aug 3, 2019

let locale = en;
switch (language) {
case "en":
case "en_US":

This comment has been minimized.

Copy link
@DenisCarriere

DenisCarriere Aug 3, 2019

Owner

going to modify these changes

It's actually supposed to be en-US

Based on the text strings provided by navigator.languages

image

This comment has been minimized.

Copy link
@DenisCarriere

DenisCarriere Aug 3, 2019

Owner

Or we set language.toLocaleLowerCase()

This comment has been minimized.

Copy link
@Favna

Favna Aug 3, 2019

Author Contributor

Oh, I didn't know it matches on navigator.languages, I'll revert some changes and create a new PR

This comment has been minimized.

Copy link
@Favna

Favna Aug 3, 2019

Author Contributor

Oh I see you already added a commit to cover this? So no more changes needed?

I did lowercase because it makes for easier matching and I thought it was only on what you give in the drawLocales function, but you seem to give me the impression that leaflet-draw calls the drawLocales function interanally with navigator.language as well?

Anyway, .toLocaleLowerCase() should work I guess
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.