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

Don't modify user-schemas in Material Object Renderer #1871

Merged
merged 1 commit into from
Apr 6, 2022

Conversation

LukasBoll
Copy link
Contributor

Don't modify user-schemas in Material Object Renderer

closes #1712

Signed-off-by: Lukas Boll lukas-bool@web.de

@coveralls
Copy link

coveralls commented Jan 15, 2022

Coverage Status

Coverage remained the same at 84.29% when pulling 77327d5 on LukasBoll:fix/modify_user_schemal into d65d7eb on eclipsesource:master.

@sdirix sdirix requested a review from max-elia January 19, 2022 07:53
Copy link
Contributor

@max-elia max-elia left a comment

Choose a reason for hiding this comment

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

The changes LGTM.
Tested it and it works as expected.

packages/material/src/complex/MaterialObjectRenderer.tsx Outdated Show resolved Hide resolved
packages/core/src/generators/uischema.ts Outdated Show resolved Hide resolved
@LukasBoll
Copy link
Contributor Author

Thanks! I updated the code accordingly.

@@ -33,7 +33,8 @@ export const Generate: {
jsonSchema: JsonSchema,
layoutType?: string,
prefix?: string,
rootSchema?: JsonSchema
rootSchema?: JsonSchema,
label?: string
Copy link
Member

Choose a reason for hiding this comment

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

Can we remove this again?

const detailUiSchema = useMemo(
() =>
findUISchema(
uischemas,
schema,
uischema.scope,
path,
'Group',
() => Generate.uiSchema(schema, 'Group', undefined, undefined, label),
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer assigning the label as part of the generator

@LukasBoll LukasBoll force-pushed the fix/modify_user_schemal branch 2 times, most recently from 1b6de6e to c874f4c Compare February 8, 2022 01:35
Comment on lines 65 to 66
if (isEmpty(path)) {
detailUiSchema.type = 'VerticalLayout';
Copy link
Member

Choose a reason for hiding this comment

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

We should also remove this modification ;)

closes eclipsesource#1712

Signed-off-by: Lukas Boll lukas-bool@web.de
@LukasBoll
Copy link
Contributor Author

Thank you, I added your suggestion!

@LukasBoll LukasBoll requested a review from sdirix April 2, 2022 17:04
@sdirix sdirix merged commit 67b036e into eclipsesource:master Apr 6, 2022
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.

Don't modify user-schemas in Material Object Renderer
4 participants