-
Notifications
You must be signed in to change notification settings - Fork 295
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(localize,form-core): wait for localize before updating feedback #1447
Conversation
🦋 Changeset detectedLatest commit: 8a76664 The changes in this PR will be included in the next version bump. This PR includes changesets to release 25 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
await el.updateComplete; | ||
await el.feedbackComplete; | ||
expect(el.label).to.equal('Tekst'); | ||
expect(_feedbackNode.feedbackData?.[0].message).to.equal('Vul een Tekst in.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was failing because "Tekst" was still "Text" from the old locale. Now that it awaits localize.loadingComplete, it will get the right localized label in the feedback message
36d3c46
to
8a76664
Compare
Nice catch and fix. Maybe we can solve it in a way such that ValidateMixin (and with that the complete form-core package) becomes i18n system agnostic? We should actually get rid of the lines here: https://github.com/ing-bank/lion/blob/master/packages/form-core/src/validate/ValidateMixin.js#L256 Maybe we can solve it with a controller, like this? form-core: localization system agnostic// ValidateManager
export class ValidateManager {
addLocalizeController(ctrl) {
this.localizeControllers.push(ctrl);
}
}
ValidateManager.localizeControllers = [];
// ValidateMixin
import { ValidateManager } from ‘./ValidateManager.js’;
...
constructor() {
ValidateManager.localizeControllers.forEach(ctrl => {
ctrl(this);
});
}
... validate-messages
import { ValidateManager } from ‘@lion/form-core’;
import {localize} from ‘@lion/localize;
ValidateManager.addLocalizeController(class LionLocalizeController {
...
hostConnected() {
localize.addEventListener('localeChanged', this._updateValidateMixinFeedbackComponent);
}
...
async _updateValidateMixinFeedbackComponent(ev) {
await localize.loadingComplete;
this.host._updateFeedbackComponent(ev);
}
}); |
If the proposal is a bit too much and you need the fix on a short term, we can also merge this fix and add the proposal as an issue/idea 👍 |
fixes #1446