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

Fix vanilla ArrayControl addItem function #1957

Merged
merged 1 commit into from
Jun 21, 2022

Conversation

eneufeld
Copy link
Member

The vanilla ArrayControl was wrapping the addItem in a function,
whereas the addItem function is already wrapped. So the onClick
can directly be bound to addItem.

Fixes #1948

@coveralls
Copy link

coveralls commented Jun 20, 2022

Coverage Status

Coverage remained the same at 84.098% when pulling 2b96271 on eneufeld:fix/1948 into 413b976 on eclipsesource:master.

Copy link
Member

@sdirix sdirix left a comment

Choose a reason for hiding this comment

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

Looks good! Left some comments.

packages/vanilla/src/complex/array/index.ts Show resolved Hide resolved
packages/examples/src/examples/1948.ts Outdated Show resolved Hide resolved
packages/examples/src/examples/1948.ts Outdated Show resolved Hide resolved
The vanilla ArrayControl was wrapping the `addItem` in a function,
whereas the `addItem` function is already wrapped. So the `onClick`
can directly be bound to `addItem`. The wrong binding lead to the
add functionality not working.

Increase rank of the ArrayControl so that it behaves the same as
for the react material renderers.

Add example to demonstrate different selections of renderers based
on the existince of a const property in the schema.

Fix wrong selection of array layout if type is missing.

Fixes eclipsesource#1948
Copy link
Member

@sdirix sdirix left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!

@sdirix sdirix merged commit 31972a5 into eclipsesource:master Jun 21, 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.

Unable to add list element
3 participants