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 web component i18n issues #2585

Merged

Conversation

sascha-karnatz
Copy link
Contributor

@sascha-karnatz sascha-karnatz commented Sep 28, 2023

What is this pull request for?

  • fix intialization issues if the web components will be rendered before the Alchemy - object is available
  • make the i18n function accessible for other modules
  • remove the Alchemy - object usage in web components
  • add web components to styleguide

Checklist

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have added tests to cover this change

@sascha-karnatz sascha-karnatz requested a review from a team September 28, 2023 12:29
Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

This looks good. Just some comments.

app/views/alchemy/admin/pages/_form.html.erb Outdated Show resolved Hide resolved
app/views/layouts/alchemy/admin.html.erb Show resolved Hide resolved
app/javascript/alchemy_admin/i18n.js Outdated Show resolved Hide resolved
@sascha-karnatz sascha-karnatz force-pushed the web-component-intialization-issues branch 3 times, most recently from 959f146 to fac1888 Compare September 29, 2023 09:58
@sascha-karnatz sascha-karnatz force-pushed the web-component-intialization-issues branch 3 times, most recently from 3b41303 to 916e50d Compare September 29, 2023 11:49
@tvdeyen tvdeyen changed the title Web component intialization issues Fix web component i18n issues Sep 29, 2023
Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

These are really great changes. There is one thing that needs to be removed, though.

app/javascript/alchemy_admin/i18n.js Outdated Show resolved Hide resolved
lib/generators/alchemy/install/files/all.js Outdated Show resolved Hide resolved
@sascha-karnatz sascha-karnatz force-pushed the web-component-intialization-issues branch 3 times, most recently from 400b24e to aac6c68 Compare October 6, 2023 08:43
Add alchemy_i18n gem to test multiple translations in out dummy app.
Export translation and currentLocale function. Move the default translations into locales/en.js and import them directly into the i18n.js to have the translations in place and prevent raise conditions. Remove Alchemy.locale and use the lang attribute of html - element.
remove the Alchemy object from alchemy-tinymce and alchemy-datepicker and use i18n function instead. This reduce the dependencies and prevents raise conditions.
Migrate the character counter component to the default AlchemyHtmlElemenet and rename the max character - attribute. Also removed the Alchemy.t - function and replaced it by the default translate function to prevent raise conditions. Added a spec file to test the component.
Update our styleguide to use our web components for tinymce, character count, and datepicker.
Reload the admin interface completely after a language change to have a proper initialization of locales and the usage of the i18n translate function inside of our web components.
@sascha-karnatz sascha-karnatz force-pushed the web-component-intialization-issues branch from aac6c68 to b387b27 Compare October 6, 2023 09:37
Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

🥳

@tvdeyen tvdeyen merged commit 214ed21 into AlchemyCMS:main Oct 6, 2023
17 checks passed
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