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

feat: use XBlockI18NService js translations #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ahmadabuwardeh
Copy link

@ahmadabuwardeh ahmadabuwardeh commented Jan 21, 2024

@ahmadabuwardeh ahmadabuwardeh self-assigned this Jan 21, 2024
@ahmadabuwardeh ahmadabuwardeh force-pushed the feature/delete_transfix branch 2 times, most recently from 78ed4ab to a626381 Compare January 21, 2024 18:40
@OmarIthawi
Copy link

@ahmadabuwardeh would you mind fixing the conflicts and changelog? Thanks!!

@ahmadabuwardeh ahmadabuwardeh force-pushed the feature/delete_transfix branch 2 times, most recently from 16bf96a to 202cff1 Compare January 24, 2024 14:16
@OmarIthawi
Copy link

@ahmadabuwardeh please revert the gettext and i18n_service changes. The pull request needs to be atomic:

We only need to rebase over master to fix conflicts and use the XBlockI18nService.

If we'd like to refactor the code it should be in a separate commit, and ideally separate pull request.

@ahmadabuwardeh ahmadabuwardeh force-pushed the feature/delete_transfix branch 2 times, most recently from 2c8da86 to b9afb1e Compare January 27, 2024 12:57
@OmarIthawi OmarIthawi changed the title use XBlockI18NService js translations feat: use XBlockI18NService js translations Jan 29, 2024
return self.runtime.local_resource_url(self, deprecated_url)

return None

def validate_field_data(self, validation, data):

Choose a reason for hiding this comment

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

@ahmadabuwardeh thanks again!

Please revert all refactoring changes to this pull request. We only need the _get_statici18n_js_url and _get_deprecated_i18n_js_url.

@@ -31,6 +31,14 @@ def _(text):
return text


def ngettext_fallback(text_singular, text_plural, number):
Copy link

@OmarIthawi OmarIthawi Jan 29, 2024

Choose a reason for hiding this comment

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

@ahmadabuwardeh Looking at this change it makes sense to have in a separate pull request.

DummyTranslationService is roughly similar to xblock.xblock.runtime.NullI18nService which can be used.

However, I'd like to understand the motivation behind it and if it's crucial for this pull request.

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