wix-incubator / autoviews

A library for building user interfaces with JSON schemas, react components and data
MIT License
12 stars 4 forks source link

Suggestion to add `AutoGroups` and `AutoHeaders`, similar to `AutoFields` and `AutoItems` #57

Open yoavaa opened 2 years ago

yoavaa commented 2 years ago

Is there an existing issue for this?

The problem

rendering table headers and groups is too complex and requires direct usage of too many low level functions and concepts, which should be encapsulated.

Describe the solution you'd like

AutoHeaders

<AutoHeaders {...props}>
  {(header, i) => <TableCell key={i}>{header}</TableCell>}
</AutoHeaders>

used to render field names in the same order as AutoFields will render the components. The provided children callback is called for each rendered component with the field title and index.

AutoGroups

<AutoGroups {...props}>
{(name, children) => (
  <Form.Group className="shadow p-3 mb-2 bg-light rounded" key={name}>
    {children}
  </Form.Group>
)}
</AutoGroups>

The equivalent of AutoFields but also supports groups from UISchema.

The callback is called with the group name and the result of AutoFields for that group. The component by default also renders the UNGROUPED group, which we can decide to control using another prop

Example here - https://codesandbox.io/s/autoviews-demo-forked-dzc83n?file=/src/autoExtensions.tsx

Describe alternatives you've considered

No response

Fer0x commented 2 years ago

While AutoGroups seems to be legal, AutoHeaders is something too specific, related to Table representation, which core not limited to. We could call it AutoTitles, or try to make it more universal, like taking additional prop with field key which would be mapped to children.

tyv commented 2 years ago

Both components are nice to have thing to hide direct usage of low level utils, but not yet sure they are the similar to AutoItems and AutoFields as they are also very low level.

Let's have a call about this proposal and try to think together on the best api. The problem though is very relevant.

yoavaa commented 2 years ago

The reason I advocate for those two components is that is separates the higher level elements (auto items, auto groups, etc) with the low level concepts, making documentation way simpler and understanding the product simpler for a new user

yoavaa commented 2 years ago

Adding more notes -

I can easily see how we can make AutoHeaders more generic by turning it into AutoSchemaFields which basically accepts the JSONSchema node for the map function.

The example from above turns into the following

<AutoSchemaFields {...props}>
  {(schemaField, i) => <TableCell key={i}>{schemaField.title}</TableCell>}
</AutoSchemaFields>
tyv commented 2 years ago

Then why do we need a React component AutoSchemaFields, it could be just utility.

tyv commented 2 years ago

Regarding AutoHeaders. Here is how I see it today. It is combination of 2 things:

  1. some utility that converts schema to the data considering UISchema (remove hidden, order, etc..), builds a schema for that converted schema automatically.
  2. Ordinar AutoView usage.

Example: https://codesandbox.io/s/autoheaders-7xss6o?file=/src/AutoHeaders.tsx


Why I think it is important to to have schema=>data conversion?

  1. it is in fact what is needed
  2. there is no way with headers auto-events would work properly, so it should be clear that schema and data are new and if it is needed to have onclicks/onchange(s): they should consider this transformation.

I don't see any better way. This looks good I think.

@yoavaa and @Fer0x plz take a look

yoavaa commented 2 years ago

I want to understand - your suggestion is basically the structure below

The second part is great, but the first is too strange, and the two schemas convertedSchema and convertedSchema2Data are again very strange.

The problem with the below is that the intent gets lost with a too verbose code.

  1. define the repo

    const tableHeadRepo = new ComponentsRepo("tableHeadRepo")
    .register("array", {
    name: "tableHeaed",
    component: (props) => (
      <thead>
        <AutoItems {...props} />
      </thead>
    )
    })
    .register("object", {
    name: "tableHeadCell",
    component: (props) => <th>{props.data.schema.title}</th>
    });
  2. the table component is then

    new ComponentsRepo("complex")
    .register("array", {
      name: "table",
      component: (props) => {
        return (
          <table>
            <RepositoryProvider components={headRepo}>
              <AutoView schema={convertedSchema} data={convertedSchema2Data} />
            </RepositoryProvider>
            <tbody>
              <AutoItems {...props} render={(item) => <tr>{item}</tr>} />
            </tbody>
          </table>
        );
      }
    })
    .register("object", {
      name: "tableRow",
      component: (props) => (
        <AutoFields {...props} render={(item) => <td>{item}</td>} />
      )
    })
yoavaa commented 2 years ago

What I am trying to say is that it looks too complicated.

What I am looking for is something like

  new ComponentsRepo("complex")
    .register("array", {
      name: "table",
      component: (props) => {
        return (
          <table>
            <schemaAsData {...props} path={items} > 
              <AutoItems {...props} > // the items here are the schema fields of the item
            </schemaAsData>
            <tbody>
              <AutoItems {...props} render={(item) => <tr>{item}</tr>} />
            </tbody>
          </table>
        );
      }
    })
Fer0x commented 2 years ago

@yoavaa in terms of AutoViews if data is completely different so it should be different AutoView scope with own props /repo/repositoryContext. Code might be rewritten to something like this:

  new ComponentsRepo("complex")
    .register("array", {
      name: "table",
      component: (props) => {
        return (
          <table>
            <thead>
              <SchemaAsData {...props} path="/items"> 
                 {headerProps => <AutoItems {...headerProps} render={item => <th>{item}</th>} />}
              </SchemaAsData>
            </thead>
            <tbody>
              <AutoItems {...props} render={(item) => <tr>{item}</tr>} />
            </tbody>
          </table>
        );
      }
    })

But anyway, <SchemaAsData /> should contain RepositoryProvider with some different repo.

tyv commented 2 years ago

@yoavaa the problem in your example is auto-passing props. which is impossible, as the data and schema are different. what if prop contains onClick, or other things that relates to original data structure.

yoavaa commented 2 years ago

I think we are getting to an interesting direction.

The concept of schema as data, which replaces the props and can use a different component repo looks like a good ackaging for headers.

Given we translate schema to data, we can also translate using the UI schema for field ordering and filters, or create an equivalent UI schema to do so.

Anyway, it feels like something that takes a good form