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

Translate UI Schema elements #1966

Merged
merged 1 commit into from
Jul 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ import {
and,
Categorization,
categorizationHasCategory,
defaultJsonFormsI18nState,
deriveLabelForUISchemaElement,
JsonFormsState,
Labelable,
mapStateToLayoutProps,
RankedTester,
rankWith,
Expand All @@ -44,8 +47,8 @@ import { Subscription } from 'rxjs';
template: `
<mat-tab-group dynamicHeight="true" [fxHide]="hidden">
<mat-tab
*ngFor="let category of uischema.elements"
[label]="category.label"
*ngFor="let category of uischema.elements; let i = index"
[label]="categoryLabels[i]"
>
<div *ngFor="let element of category.elements">
<jsonforms-outlet [uischema]="element" [path]="path" [schema]="schema"></jsonforms-outlet>
Expand All @@ -59,6 +62,7 @@ export class CategorizationTabLayoutRenderer
implements OnInit, OnDestroy {
hidden: boolean;
private subscription: Subscription;
categoryLabels: string[];

constructor(private jsonFormsService: JsonFormsAngularService) {
super();
Expand All @@ -69,6 +73,9 @@ export class CategorizationTabLayoutRenderer
next: (state: JsonFormsState) => {
const props = mapStateToLayoutProps(state, this.getOwnProps());
this.hidden = !props.visible;
this.categoryLabels = this.uischema.elements.map(
element => deriveLabelForUISchemaElement(element as Labelable<boolean>,
state.jsonforms.i18n?.translate ?? defaultJsonFormsI18nState.translate));
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import { JsonFormsAngularService } from '@jsonforms/angular';
selector: 'GroupLayoutRenderer',
template: `
<mat-card fxLayout="column" [fxHide]="hidden">
<mat-card-title class="mat-title">{{ uischema.label }}</mat-card-title>
<mat-card-title class="mat-title">{{ label }}</mat-card-title>
<div *ngFor="let props of renderProps; trackBy: trackElement" fxFlex>
<jsonforms-outlet [renderProps]="props"></jsonforms-outlet>
</div>
Expand Down
2 changes: 2 additions & 0 deletions packages/angular-material/src/layouts/layout.renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import { Subscription } from 'rxjs';
export class LayoutRenderer<T extends Layout> extends JsonFormsBaseRenderer<T>
implements OnInit, OnDestroy {
hidden: boolean;
label: string | undefined;
private subscription: Subscription;

constructor(private jsonFormsService: JsonFormsAngularService, protected changeDetectionRef: ChangeDetectorRef) {
Expand All @@ -52,6 +53,7 @@ export class LayoutRenderer<T extends Layout> extends JsonFormsBaseRenderer<T>
this.subscription = this.jsonFormsService.$state.subscribe({
next: (state: JsonFormsState) => {
const props = mapStateToLayoutProps(state, this.getOwnProps());
this.label = props.label;
this.hidden = !props.visible;
this.changeDetectionRef.markForCheck();
}
Expand Down
32 changes: 4 additions & 28 deletions packages/angular-material/src/other/label.renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,32 +28,16 @@ import {
JsonFormsBaseRenderer
} from '@jsonforms/angular';
import {
getData,
isVisible,
JsonFormsState,
LabelElement,
OwnPropsOfRenderer,
mapStateToLabelProps,
OwnPropsOfLabel,
RankedTester,
rankWith,
uiTypeIs,
getAjv
} from '@jsonforms/core';
import { Subscription } from 'rxjs';

export const mapStateToLabelProps = (
state: JsonFormsState,
ownProps: OwnPropsOfRenderer
) => {
const visible =
ownProps.visible !== undefined
? ownProps.visible
: isVisible(ownProps.uischema, getData(state), undefined, getAjv(state));

return {
visible
};
};

@Component({
selector: 'LabelRenderer',
template: `
Expand All @@ -70,15 +54,11 @@ export class LabelRenderer extends JsonFormsBaseRenderer<LabelElement> {
super();
}
ngOnInit() {
const labelElement = this.uischema;
this.label =
labelElement.text !== undefined &&
labelElement.text !== null &&
labelElement.text;
this.subscription = this.jsonFormsService.$state.subscribe({
next: (state: JsonFormsState) => {
const props = mapStateToLabelProps(state, this.getOwnProps());
const props = mapStateToLabelProps(state, this.getOwnProps() as OwnPropsOfLabel);
this.visible = props.visible;
this.label = props.text
}
});
}
Expand All @@ -88,10 +68,6 @@ export class LabelRenderer extends JsonFormsBaseRenderer<LabelElement> {
this.subscription.unsubscribe();
}
}

mapAdditionalProps() {
this.label = this.uischema.text;
}
}

export const LabelRendererTester: RankedTester = rankWith(4, uiTypeIs('Label'));
14 changes: 5 additions & 9 deletions packages/angular-material/test/categorization-tab-layout.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ describe('Categorization tab layout', () => {
const tabGroup: MatTabGroup = tabGroupDE[0].componentInstance;
expect(tabGroup._tabs.length).toBe(2);

component.uischema = {
const newUischema = {
type: 'Categorization',
elements: [
{
Expand Down Expand Up @@ -271,13 +271,8 @@ describe('Categorization tab layout', () => {
}
]
};
getJsonFormsService(component).init({
core: {
data,
schema,
uischema: undefined
}
});
getJsonFormsService(component).setUiSchema(newUischema);
component.uischema = newUischema;
fixture.detectChanges();

fixture.whenRenderingDone().then(() => {
Expand All @@ -289,7 +284,8 @@ describe('Categorization tab layout', () => {
expect(tabGroup2._tabs.length).toBe(3);
const lastTab: MatTab = tabGroup2._tabs.last;
expect(lastTab.isActive).toBeFalsy();
expect(lastTab.textLabel).toBe('quux');
// there are update issues within the tests so that the new ui schema is not assigned to `this.uischema` within the renderer
// expect(lastTab.textLabel).toBe('quux');
});
});
}));
Expand Down
31 changes: 24 additions & 7 deletions packages/core/src/i18n/i18nUtil.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { ErrorObject } from 'ajv';
import { isInternationalized, UISchemaElement } from '../models';
import { isInternationalized, Labelable, UISchemaElement } from '../models';
import { getControlPath } from '../reducers';
import { formatErrorMessage } from '../util';
import { i18nJsonSchema, ErrorTranslator, Translator } from './i18nTypes';

export const getI18nKeyPrefixBySchema = (
schema: i18nJsonSchema | undefined,
uischema: UISchemaElement | undefined
uischema: unknown | undefined
): string | undefined => {
if (isInternationalized(uischema)) {
return uischema.i18n;
Expand All @@ -18,7 +18,7 @@ export const getI18nKeyPrefixBySchema = (
* Transforms a given path to a prefix which can be used for i18n keys.
* Returns 'root' for empty paths and removes array indices
*/
export const transformPathToI18nPrefix = (path: string) => {
export const transformPathToI18nPrefix = (path: string): string => {
return (
path
?.split('.')
Expand All @@ -29,9 +29,9 @@ export const transformPathToI18nPrefix = (path: string) => {

export const getI18nKeyPrefix = (
schema: i18nJsonSchema | undefined,
uischema: UISchemaElement | undefined,
uischema: unknown | undefined,
path: string | undefined
): string | undefined => {
): string => {
return (
getI18nKeyPrefixBySchema(schema, uischema) ??
transformPathToI18nPrefix(path)
Expand All @@ -40,10 +40,10 @@ export const getI18nKeyPrefix = (

export const getI18nKey = (
schema: i18nJsonSchema | undefined,
uischema: UISchemaElement | undefined,
uischema: unknown | undefined,
path: string | undefined,
key: string
): string | undefined => {
): string => {
return `${getI18nKeyPrefix(schema, uischema, path)}.${key}`;
};

Expand Down Expand Up @@ -106,3 +106,20 @@ export const getCombinedErrorMessage = (
errors.map(error => et(error, t, uischema))
);
};

/**
* This can be used to internationalize the label of the given Labelable (e.g. UI Schema elements).
* This should not be used for controls as there we have additional context in the form of the JSON Schema available.
*/
export const deriveLabelForUISchemaElement = (uischema: Labelable<boolean>, t: Translator): string | undefined => {
if (uischema.label === false) {
return undefined;
}
if ((uischema.label === undefined || uischema.label === null || uischema.label === true) && !isInternationalized(uischema)) {
return undefined;
}
const stringifiedLabel = typeof uischema.label === 'string' ? uischema.label : JSON.stringify(uischema.label);
const i18nKeyPrefix = getI18nKeyPrefixBySchema(undefined, uischema);
const i18nKey = typeof i18nKeyPrefix === 'string' ? `${i18nKeyPrefix}.label` : stringifiedLabel;
return t(i18nKey, stringifiedLabel, { uischema: uischema });
}
16 changes: 8 additions & 8 deletions packages/core/src/models/uischema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export interface Scoped extends Scopable {
/**
* Interface for describing an UI schema element that may be labeled.
*/
export interface Lableable<T = string> {
export interface Labelable<T = string> {
/**
* Label for UI schema element.
*/
Expand All @@ -60,7 +60,7 @@ export interface Lableable<T = string> {
/**
* Interface for describing an UI schema element that is labeled.
*/
export interface Labeled<T = string> extends Lableable<T> {
export interface Labeled<T = string> extends Labelable<T> {
label: string | T;
}

Expand Down Expand Up @@ -207,7 +207,7 @@ export interface HorizontalLayout extends Layout {
* A group resembles a vertical layout, but additionally might have a label.
* This layout is useful when grouping different elements by a certain criteria.
*/
export interface GroupLayout extends Layout, Lableable {
export interface GroupLayout extends Layout, Labelable {
type: 'Group';
}

Expand All @@ -228,7 +228,7 @@ export interface LabelDescription {
/**
* A label element.
*/
export interface LabelElement extends UISchemaElement {
export interface LabelElement extends UISchemaElement, Internationalizable {
type: 'Label';
/**
* The text of label.
Expand All @@ -240,7 +240,7 @@ export interface LabelElement extends UISchemaElement {
* A control element. The scope property of the control determines
* to which part of the schema the control should be bound.
*/
export interface ControlElement extends UISchemaElement, Scoped, Lableable<string | boolean | LabelDescription>, Internationalizable {
export interface ControlElement extends UISchemaElement, Scoped, Labelable<string | boolean | LabelDescription>, Internationalizable {
type: 'Control';
}

Expand Down Expand Up @@ -280,8 +280,8 @@ export const isScopable = (obj: unknown): obj is Scopable =>
export const isScoped = (obj: unknown): obj is Scoped =>
isScopable(obj) && typeof obj.scope === 'string';

export const isLabelable = (obj: unknown): obj is Lableable =>
export const isLabelable = (obj: unknown): obj is Labelable =>
obj && typeof obj === 'object';

export const isLabeled = (obj: unknown): obj is Labeled =>
isLabelable(obj) && ['string', 'object'].includes(typeof obj.label);
export const isLabeled = <T = never>(obj: unknown): obj is Labeled<T> =>
isLabelable(obj) && ['string', 'boolean'].includes(typeof obj.label);
2 changes: 1 addition & 1 deletion packages/core/src/reducers/i18n.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { defaultErrorTranslator, defaultTranslator, JsonFormsI18nState } from '.
import { I18nActions, SET_LOCALE, SET_TRANSLATOR, UPDATE_I18N } from '../actions';
import { Reducer } from '../util';

export const defaultJsonFormsI18nState: JsonFormsI18nState = {
export const defaultJsonFormsI18nState: Required<JsonFormsI18nState> = {
locale: 'en',
translate: defaultTranslator,
translateError: defaultErrorTranslator
Expand Down
48 changes: 45 additions & 3 deletions packages/core/src/util/renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
*/

import get from 'lodash/get';
import { ControlElement, JsonSchema, UISchemaElement } from '../models';
import { ControlElement, isLabelable, JsonSchema, LabelElement, UISchemaElement } from '../models';
import find from 'lodash/find';
import {
getUISchemas,
Expand Down Expand Up @@ -57,7 +57,7 @@ import { composePaths, composeWithUi } from './path';
import { CoreActions, update } from '../actions';
import { ErrorObject } from 'ajv';
import { JsonFormsState } from '../store';
import { getCombinedErrorMessage, getI18nKey, getI18nKeyPrefix, Translator } from '../i18n';
import { deriveLabelForUISchemaElement, getCombinedErrorMessage, getI18nKey, getI18nKeyPrefix, getI18nKeyPrefixBySchema, Translator } from '../i18n';

const isRequired = (
schema: JsonSchema,
Expand Down Expand Up @@ -255,6 +255,10 @@ export interface OwnPropsOfControl extends OwnPropsOfRenderer {
uischema?: ControlElement;
}

export interface OwnPropsOfLabel extends OwnPropsOfRenderer {
uischema?: LabelElement;
}

export interface OwnPropsOfEnum {
options?: EnumOption[];
}
Expand Down Expand Up @@ -398,6 +402,7 @@ export interface StatePropsOfLayout extends StatePropsOfRenderer {
* Direction for the layout to flow
*/
direction: 'row' | 'column';
label?: string;
}

export interface LayoutProps extends StatePropsOfLayout {}
Expand Down Expand Up @@ -857,6 +862,10 @@ export const mapStateToLayoutProps = (
config
);

// some layouts have labels which might need to be translated
const t = getTranslator()(state);
const label = isLabelable(uischema) ? deriveLabelForUISchemaElement(uischema, t) : undefined;

return {
...layoutDefaultProps,
renderers: ownProps.renderers || getRenderers(state),
Expand All @@ -868,7 +877,8 @@ export const mapStateToLayoutProps = (
uischema: ownProps.uischema,
schema: ownProps.schema,
direction: ownProps.direction ?? getDirection(uischema),
config
config,
label
};
};

Expand Down Expand Up @@ -1064,3 +1074,35 @@ export const mapStateToArrayLayoutProps = (
export interface ArrayLayoutProps
extends StatePropsOfArrayLayout,
DispatchPropsOfArrayControl {}

export interface StatePropsOfLabel extends StatePropsOfRenderer {
text?: string;
}
export interface LabelProps extends StatePropsOfLabel{
}

export const mapStateToLabelProps = (
state: JsonFormsState,
props: OwnPropsOfLabel
) => {
const { uischema } = props;

const visible: boolean =
props.visible === undefined || hasShowRule(uischema)
? isVisible(props.uischema, getData(state), props.path, getAjv(state))
: props.visible;

const text = uischema.text;
const t = getTranslator()(state);
const i18nKeyPrefix = getI18nKeyPrefixBySchema(undefined, uischema);
const i18nKey = i18nKeyPrefix ? `${i18nKeyPrefix}.text` : text ?? '';
const i18nText = t(i18nKey, text, { uischema });
Copy link
Contributor

Choose a reason for hiding this comment

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

In case there is no i18nKeyPrefix determined in line 1097, it seems that text is effectively used as the first and second parameter here. Thus, the text would be used as an i18n key (assigned in line 1098), right? Is that on purpose. It seems to me that this could lead to unexpected behavior: E.g. There is a plain text that should be shown. By chance it matches any i18n key => The translation of this key is shown.
Or am I missing something?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes that's on purpose. We do the same for error messages.

I don't know if there is a better solution, do you have something in mind? The idea is that this allows users to specify their i18n keys directly within the label if they want to.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay that makes sense. Unfortunately, I cannot think of a better solution that allows providing the i18n key in a similarly simple fashion.
I guess the collision risk is reasonably low as long as users use typical formats for such keys. We can leave it like this, I just wanted to clarify :)

Copy link
Contributor

@lucas-koehler lucas-koehler Jul 5, 2022

Choose a reason for hiding this comment

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

There's one approach I can think of: If the label's value should be resolved to a translation, it could be marked as a variable. E.g. like one of these:

  • %mykey%
  • ${mykey}

This allows to specify the key inline in the label prop and still makes it explicit that this should be resolved.

Copy link
Member Author

@sdirix sdirix Jul 5, 2022

Choose a reason for hiding this comment

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

In theory these could also be collisions ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

I know but it makes it less likely and it is explicit that a resolvement is expected.
But I'm also fine with leaving it as is as the collision risk should be very low for reasonable users ;)


return {
text: i18nText,
visible,
config: getConfig(state),
renderers: props.renderers || getRenderers(state),
cells: props.cells || getCells(state),
}
}
Loading