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

EnumArray Renderer not working with $refs #2192

Closed
ananyabarthakur opened this issue Oct 18, 2023 · 17 comments · Fixed by #2204
Closed

EnumArray Renderer not working with $refs #2192

ananyabarthakur opened this issue Oct 18, 2023 · 17 comments · Fixed by #2204

Comments

@ananyabarthakur
Copy link

Describe the bug

The EnumArray renderer works without $refs in the schema, but with $refs, those EnumOptions do not seem to get handled.

The 'options' prop should have returned the list of enum options, but comes back undefined. There seems to be no other prop to use other than 'options' in ControlProps, OwnPropsOfEnum, DispatchPropsOfMultiEnumControl.

Here is the sample schema:
schemaWithRef = {
$schema: 'http://json-schema.org/draft-07/schema#',
additionalProperties: false,
type: 'object',
definitions: {
Partner: {
enum: ['DeutscheTelekom', 'GoDaddy', 'Liquid', 'Rogers'],
type: 'string'
}
},
properties: {
partner: {
items: {
$ref: '#/definitions/Partner'
},
type: 'array'
}
}
}

Expected behavior

The expected behavior is that the 'options' prop should return ['DeutscheTelekom', 'GoDaddy', 'Liquid', 'Rogers'], but instead returns undefined.

Steps to reproduce the issue

Dispatch JSON forms with the given sample schema.

Screenshots

No response

In which browser are you experiencing the issue?

Edge 118

Which Version of JSON Forms are you using?

v3.1.0

Framework

React

RendererSet

No response

Additional context

FluentUI

@sdirix
Copy link
Member

sdirix commented Oct 18, 2023

HI @ananyabarthakur,

Thanks for the report. In which renderer set does this problem occur? It works fine for me in the Material UI renderer set:

image

@ananyabarthakur
Copy link
Author

ananyabarthakur commented Oct 18, 2023 via email

@sdirix
Copy link
Member

sdirix commented Oct 18, 2023

Hi @ananyabarthakur,

Where is this FluentUI renderer coming from? We don't offer a renderer set with FluentUI.

An enum in JSON Schema is necessarily a single selection. To allow for multi selection you can for example model an array consisting of enum values and marking the array as to contain unique elements. We support this already for some of the renderer sets, see here for the docs.

@ananyabarthakur
Copy link
Author

Hello @sdirix, I am using React.

@ananyabarthakur
Copy link
Author

Hi @sdirix , for more information, I am creating a custom renderer using React, and here is the way I get the props + begin my component: interface EnumArrayControlProps
extends ControlProps,
OwnPropsOfEnum,
DispatchPropsOfMultiEnumControl {}
export const EnumArrayControl: React.FC = ({
visible,
errors,
path,
options,
data,
addItem,
removeItem,
label,
required,
enabled,
schema,
rootSchema
}) => {

Would you be able to share the component code that works on your end?

@sdirix
Copy link
Member

sdirix commented Oct 19, 2023

The source of all our renderers is available in this repository, so you can simply check how they are implemented, e.g. see here.

@arunarivarasan
Copy link

arunarivarasan commented Oct 19, 2023

"Thank you! We used similar code from our Fluent UI style guide instead of Material UI. For some reason, the 'Options' parameter is undefined if the schema contains a '$ref' for an enum within an array of enum renderer.

This is not working:

 {
$schema: 'http://json-schema.org/draft-07/schema#',
additionalProperties: false,
type: 'object',
definitions: {
Partner: {
enum: ['DeutscheTelekom', 'GoDaddy', 'Liquid', 'Rogers'],
type: 'string'
}
},
properties: {
  partner: {
    items: {
      $ref: '#/definitions/Partner'
    },
    type: 'array'
  }
}

This is working:

properties: {
  partner: {
    items: {
      enum: ['DeutscheTelekom', 'GoDaddy', 'Liquid', 'Rogers'],
      type: 'string'
    },
    type: 'array'
  }
}

Somehow, the '$ref' is not resolving in the above scenario. We have built many renderers with '$ref' that are all working as expected."

These changes provide a bit more clarity and structure to your message.

@ananyabarthakur
Copy link
Author

Hello @sdirix,

I had question - your renderer MaterialEnumArrayRenderer uses an MUI Checkbox as its control. However, per the screenshot you sent and after implementing the same renderer locally, I see that it in fact renders a detail List of a single select dropdowns. How is it the case that it is not the MUI checkbox?

@sdirix
Copy link
Member

sdirix commented Oct 24, 2023

The MaterialEnumArrayRenderer is only used in very specific circumstances. The schema posted in OP is simpler, therefore a more generic renderer is used. The generic renderer just dispatches again for all properties, which in this case is a single select for each entry.

The schema needs to specify uniqueItems: true so that the renderer is applied.

Note that I just tested this and in fact the tester linked above has a bug: It doesn't check whether items is a $ref for resolving, so even when adding uniqueItems: true in the JSON Schema it will not work because of it. This should be fixed.

@ananyabarthakur
Copy link
Author

I see! Thank you for trying it out as well. So how would you fix the tester such that, if there is a $ref, we have it resolved?

@sdirix
Copy link
Member

sdirix commented Oct 24, 2023

The schemaSubPathMatches part must resolve the items schema like this tester does.

Without testing the code, it should look like this:

export const materialEnumArrayRendererTester: RankedTester = rankWith(
    5,
    and(
        uiTypeIs('Control'),
        and(
            schemaMatches(
                (schema) =>
                    hasType(schema, 'array') &&
                    !Array.isArray(schema.items) &&
                    schema.uniqueItems === true
            ),
            schemaSubPathMatches('items', (schema, rootSchema) => {
                const resolvedSchema = schema.$ref
                    ? resolveSchema(rootSchema, schema.$ref, rootSchema)
                    : schema;

                return hasOneOfItems(schema) || hasEnumItems(schema);
            })
        )
    )
);

@ananyabarthakur
Copy link
Author

I see. And does it work for you? I tried it on my end and it control fails to render due to the following error TypeError: Cannot read properties of undefined (reading 'map'). It seems like the resolver resolves the schema that is sent to it, but the schema that gets processed by the EnumArray renderer still has the $ref in it. Probably because we are not saving/sending back the resolvedschema anywhere?

@ananyabarthakur
Copy link
Author

Is there a helper function that allows for the modification of the schema for all instances of the schema (for example, editing the schema with said function would have the renderers use the modified, resolved schema instead of the original unresolved schema?

@sdirix
Copy link
Member

sdirix commented Oct 26, 2023

The testers and the renderer are independent from each other. So in the renderer the items also need to be resolved again. For renderers like the arrays we are already doing this. Here, similar to the tester, this was overlooked.

However as you are implementing a custom renderer anyway you can easily resolve them yourself in the renderer in the same way as is done in the tester.

@ananyabarthakur
Copy link
Author

Would you be able to share the link to the file of the array renderer that implements the resolving of schema? I may be looking in the wrong place (materialrenderers/complex). Thanks!!

LukasBoll added a commit to LukasBoll/jsonforms that referenced this issue Oct 30, 2023
resolve refs ind the MultiEnumControlProps mapper and tester

fixes eclipsesource#2192
@ananyabarthakur
Copy link
Author

Thank you!! It works now :) A somewhat side question, our default control is a plain text field, and now with EnumArrays working, it seems that fields without a specific control are defaulting to EnumArray instead of the text fields. EnumArrays are taking precedence over Default, even though our ranking, in order, is -
Default, EnumArrayControl, PlainText,...
It is interesting because when it is -
Default, PlainText, EnumArrayControl,..., what happens is that not only the defaults, but also actual textfields become dropdowns. I wonder if it is because we need to use cells?

@sdirix
Copy link
Member

sdirix commented Nov 1, 2023

Hi @ananyabarthakur, it seems that your tester reacts too broadly, i.e. it not only returns a high priority for the enum arrays case, but also for the other cases you mentioned. For example you might have placed an "or" check to high. I would recommend to implement some test cases for the now used tester to verify that it only returns a high priority for enum arrays and not for strings etc.

LukasBoll added a commit to LukasBoll/jsonforms that referenced this issue Nov 13, 2023
resolve refs ind the MultiEnumControlProps mapper and tester

fixes eclipsesource#2192
lucas-koehler pushed a commit that referenced this issue Nov 13, 2023
resolve refs ind the MultiEnumControlProps mapper and tester

fixes #2192
@sdirix sdirix added this to the 3.2 milestone Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants