Add context for array operation by eneufeld · Pull Request #2317 · eclipsesource/jsonforms

Conversation

@eneufeld

Currently it is very hard to figure out in the middleware what array operation happend to lead to the current array state. There are cases were this information is valuable. To support this all update actions can now have a 'context'. Here we added a specific 'UpdateArrayContext'.

Contributed on behalf of STMicroelectronics

@netlify

@coveralls

Coverage Status

coverage: 84.795% (-0.009%) from 84.804%
when pulling 2fcf638 on eneufeld:arrayContext
into c74b8ba on eclipsesource:master.

sdirix

Choose a reason for hiding this comment

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

Looks good and thanks for adding the tests. Please have a look at my comments.

sdirix

Comment on lines +80 to +81

const firstValueType = typeof context.values[0];
return context.values.every((v) => typeof v === firstValueType);

Choose a reason for hiding this comment

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

I think this check is not necessary. JSON Schema supports different types in arrays.

Choose a reason for hiding this comment

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

but we don't support this in jsonforms, so all newly added values must be of the same type
I'm fine removing this check

Choose a reason for hiding this comment

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

We don't support it in JSON Forms in our current renderers but there is no restriction for that in JSON Schema and custom/future renderers might support that.

sdirix

Currently it is very hard to figure out in the middleware what array
operation happend to lead to the current array state.
There are cases were this information is valuable.
To support this all update actions can now have a 'context'.
Here we added a specific 'UpdateArrayContext'.

Contributed on behalf of STMicroelectronics

sdirix

Choose a reason for hiding this comment

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

Works for me!

Labels