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

add a deep ui schema generator #1666

Closed

Conversation

weiner
Copy link

@weiner weiner commented Dec 13, 2020

Add a deep ui schema generator.

Motivation:
It is easier to customize the form UI in the json form editor when you can see all UI elements used and not just the one on the root level.
This PR adds a deep ui schema generator which can be used by the json form edtor / a component embedding json from editor.

Thanks for this awesome project.

…his to give a better Form uiSchema edit experiance.
@CLAassistant
Copy link

CLAassistant commented Dec 13, 2020

CLA assistant check
All committers have signed the CLA.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 88.71% when pulling 18da931 on agilegravity:deep-ui-schema-generator into cd457e1 on eclipsesource:master.

@sdirix
Copy link
Member

sdirix commented Dec 16, 2020

Thank you for the contribution!

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.

Hi @weiner, I had a look at the PR.

Regarding the code, I don't think we need a separate "deep" generator. The PR mostly consists of copied code and in the future we would need to make sure that any change is done in both the normal and the deep generator. Instead I would prefer additional parameters to the existing generator with which the behavior can be influenced.

Generally speaking I'm not sure whether we need the functionality at all. You specifically mentioned the JSON Forms editor as a use case. There this PR would serve to avoid using the object renderer, which is a plus in the current state as the object renderer is pretty basic and has a bug. However instead of avoiding the object renderer I would rather see it fixed and enhanced. Another effect of this PR is that arrays are never rendered as tables but always with nested layouts. I'm not sure whether this is preferable over the default table rendering.

So in short: I would rather improve the JSON Forms editor than adding functionality to the JSON Forms core to avoid temporary quirky use cases of the editor.

Do you agree with this reasoning? Are there other use cases where you would see a "deep" generator as useful?

return layout;
}
if (types[0] === 'array') {
const layout: Layout = createLayout('Control');
Copy link
Member

Choose a reason for hiding this comment

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

Why not use createControlElement?

if (types[0] === 'array') {
const layout: Layout = createLayout('Control');
delete layout.elements;
// @ts-ignore
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 like to avoid // @ts-ignore wherever possible

layoutType,
rootSchema
);
if (inArrayUiSchema) {
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this always truthy?

@sdirix
Copy link
Member

sdirix commented Dec 7, 2021

I'll close this PR for now. Let me know when you have further questions. In general I would be interested in a generic way to let the user influence UI Schema generation, i.e. to allow to pass a generator function to JSON Forms.

@sdirix sdirix closed this Dec 7, 2021
@mingfang
Copy link

I vote for an option to generate a "deep" UI Schema.
I'm actually perplexed by why this is not the default because the benefit of the generator is equally important for the next objects as the root.

@bangank36
Copy link

Are there other use cases where you would see a "deep" generator as useful?

@sdirix I think my use case will be exactly the reason for this deep nested generator:

  • In my project, I am in need of rendering the nested control into separate screen to match the markup of Gutenberg Navigator Component, you can also see the generated markup for the navigator and jsonform here

  • Basically, I need to render the nested prop in flat hierarchy instead of nested structure like currently, to achieve that I have introduced the NavigatorLayout WP-Builder: Introduce new NavigatorLayout bangank36/WP-Builder#19 and implemented a new ObjectRenderer to render only the NavigatorButton to navigate to the screen

  • So the challenge for me is to somehow generate whole Navigator markup inside the NavigatorProvider, I am pursuing 2 approach

    1. Generate the deep uiSchema from root layout and render all JsonFormDispatch
    2. Let the nested node generate its uischema and dispatch the JsonFormDispatch to the root layout (via new Context, or I wonder if we can take advantage of useJsonFoms hook to add additional data)
  • I can see some similar discussions on the forum here

Please give some advice if you can, thank you!!

PS1: Please check the generated markup for the whole new layout

<>
	<NavigatorScreen path="/">
		<JsonFormsDispatch />
	</NavigatorScreen>
	<NavigatorScreen path="/address">
		<JsonFormsDispatch />
	</NavigatorScreen>
	<NavigatorScreen path="/address/country">
		<JsonFormsDispatch />
	</NavigatorScreen>
	<NavigatorScreen path="/job">
		<JsonFormsDispatch />
	</NavigatorScreen>
</>

PS2: You can see my partial solution here
chrome-capture-2023-6-8

@sdirix
Copy link
Member

sdirix commented Jul 10, 2023

Hi @bangank36,

Questions like these are rather suited for the forum. Discussions within closed issues are rarely found by anyone.


First we need to clarify the purpose of the NavigatorLayout, e.g. the NavigatorLayout shall be used to render objects in which all nested properties are objects.

The nice thing about this requirement is:

  • It's easy to understand how it works
  • It's generically usable for all objects containing nested objects
  • It fits well with already existing feature set of JSON Forms

The NavigatorLayout would then just be a custom renderer for type:object. You could trigger it via the UI Schema or specify that it always shall be used for the root of the form if the schema is shaped accordingly.

The renderer could then look like this (not tested)

const NavigatorLayoutRenderer = (props) => {
  const controls = useMemo(() => generateControls(props.schema), [props.schema]);
  return (
  <NavigatorProvider initialPath="/">
      <NavigatorScreen path="/">
        <p>{{props.label}}</p>
        {{Object.keys(props.schema.properties).map(propertyKey => (
          <NavigatorButton path={`/${propertyKey}`} key={propertyKey}>
             Navigate to {{propertyKey}}.
          </NavigatorButton>
        )}}
      </NavigatorScreen>
      {{Object.keys(props.schema.properties).map(propertyKey => (
        <NavigatorScreen path={`/${propertyKey}`} key={propertyKey}>
           <JsonFormsDispatch schema={schema} uischema={controls[propertyKey]} path={props.path}/>
        </NavigatorScreen>
      )}}
  </NavigatorProvider>);
}

const generateControls = (schema) => {
  const result = {};
  Object.keys(schema.properties).forEach(key => {
    result[key] = { type: 'Control', scope: `#/properties/${encode(key)}` };
  });
  return result;
}

So I don't see any need for some deep UI Schema generation and only a single custom renderer is needed. Does this make sense to you?

Edit: Follow up discussion here: https://jsonforms.discourse.group/t/render-nested-forms-in-flat-structure/1591/5

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.

6 participants