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

mapStateToJsonFormsRendererProps should map enabled property just like react own version of the method #1884

Closed
kchobantonov opened this issue Feb 6, 2022 · 2 comments · Fixed by #1968
Milestone

Comments

@kchobantonov
Copy link
Contributor

kchobantonov commented Feb 6, 2022

Describe the bug

Currently when using different render set than react the enabled property is not preserved.

For example if we use the vuetify render set in this example (https://jsonforms-vuetify-renderers.netlify.app/#/example/main) - the "Address for Shipping T-Shirt" section is never disabled per committer checkbox.

Here is the relevant part of the react renderer set that does not use the core version of mapStateToJsonFormsRendererProps

const useJsonFormsDispatchRendererProps = (props: OwnPropsOfJsonFormsRenderer & JsonFormsReactProps) => {
const ctx = useJsonForms();
return {
schema: props.schema || ctx.core.schema,
uischema: props.uischema || ctx.core.uischema,
path: props.path || '',
enabled: props.enabled,
rootSchema: ctx.core.schema,
renderers: props.renderers || ctx.renderers,
cells: props.cells || ctx.cells,
};
}

Here is the relevant discussion on discourse https://jsonforms.discourse.group/t/jsonforms-should-propagate-enabled-property-when-mapping-render-properties/489

Expected behavior

Expected behavior is that the enabled property is preserved in its own properties so that it can be passed to lower components and be able to disable/enable them

Steps to reproduce the issue

  1. Go to 'https://jsonforms-vuetify-renderers.netlify.app/#/example/main'
  2. Click on 'Committer' checkbox - make sure that it is not checked
  3. See "Address for Shipping T-Shirt" never disabled

Screenshots

No response

In which browser are you experiencing the issue?

any

Framework

Core, Vue 2

RendererSet

Other (please specify in the Additional context field)

Additional context

https://github.com/eclipsesource/jsonforms-vuetify-renderers

Potentially we could need the visibility to be mapped as well perhaps

@kchobantonov kchobantonov changed the title mapStateToJsonFormsRendererProps should map visible property just like react own version of the method mapStateToJsonFormsRendererProps should map enabled property just like react own version of the method Feb 6, 2022
@sdirix
Copy link
Member

sdirix commented Feb 10, 2022

Hi @kchobantonov, as mentioned in the discussion. I agree, this should definitely be improved upon.

@sdirix sdirix added this to the 3.x milestone Feb 10, 2022
@sdirix sdirix linked a pull request Jul 18, 2022 that will close this issue
@sdirix sdirix modified the milestones: 3.x, 3.0 Jul 18, 2022
@sdirix
Copy link
Member

sdirix commented Jul 18, 2022

Fixed with #1968

@sdirix sdirix closed this as completed Jul 18, 2022
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.

2 participants