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

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

Open
1 task done
yoavaa opened this issue Apr 4, 2022 · 11 comments
Open
1 task done
Labels
feature-request New feature or request

Comments

@yoavaa
Copy link
Collaborator

yoavaa commented Apr 4, 2022

Is there an existing issue for this?

  • Yes, I have searched the existing issues

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

@yoavaa yoavaa added the feature-request New feature or request label Apr 4, 2022
@Fer0x
Copy link
Contributor

Fer0x commented Apr 5, 2022

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
Copy link
Collaborator

tyv commented Apr 5, 2022

  • AutoHeaders or AutoTitles it might be AutoViews when you use JSONSchema as data: fields of the object and optional titles becomes data. I think we need to explore and think about this idea a bit more. The proposed option wouldn't work for binding AutoHeaders in ComponentsRepo when AutoFields and AutoItems are bindable without necessity to define children
  • AutoGroups it is basically multiple AutoFields with ability to wrap them.

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
Copy link
Collaborator Author

yoavaa commented Apr 5, 2022

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
Copy link
Collaborator Author

yoavaa commented Apr 5, 2022

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
Copy link
Collaborator

tyv commented Apr 6, 2022

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

@tyv
Copy link
Collaborator

tyv commented Apr 18, 2022

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
Copy link
Collaborator Author

yoavaa commented Apr 27, 2022

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>
  });
  1. 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
Copy link
Collaborator Author

yoavaa commented Apr 27, 2022

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
Copy link
Contributor

Fer0x commented Apr 27, 2022

@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
Copy link
Collaborator

tyv commented Apr 28, 2022

@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
Copy link
Collaborator Author

yoavaa commented Apr 28, 2022

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

tyv added a commit that referenced this issue May 9, 2022
…th map function and rules

Function that takes object type schema, optional data, pick. omit and order rules and map callback,
returns an array of map callback results. Map callback maps list of object fields after filtering
and ordering according to pick, omit and order rules.

BREAKING CHANGE: orderFields and filter functions are deleted from the export

re #57
tyv added a commit that referenced this issue May 18, 2022
AutoHeaders component is a component that converts object `JSONSchema` to array of string data,
where each string is a field name from the object schema. AutoHeaders provides new AutoView props
based on object JSONSchema

re #57
github-actions bot pushed a commit that referenced this issue Jun 1, 2022
# [@autoviews/core-v3.0.0](https://github.com/wix-incubator/autoviews/compare/@autoviews/core-v2.0.0...@autoviews/core-v3.0.0) (2022-06-01)

### Bug Fixes

* correct error message ([ef442b5](ef442b5))

### Features

* added AutoHeaders component ([0f0ed05](0f0ed05)), closes [#57](#57)
* **objectschemaasarray utility:** converts object schema to array with map function and rules ([85b0dc8](85b0dc8)), closes [#57](#57)
* **schema:** added `prefixItems` support as tuple ([eb41410](eb41410)), closes [#90](#90)

### BREAKING CHANGES

* **schema:** `CoreSchemaMetaSchema['items']` now just has `CoreSchemaMetaSchema` type,
nextSchema function now consider this field
* **objectschemaasarray utility:** orderFields and filter functions are deleted from the export
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants