pdfme / pdfme

A TypeScript based PDF generator library, made with React.
https://pdfme.com
MIT License
2.19k stars 194 forks source link

Add ReadOnly Schemas #386

Closed hand-dot closed 5 months ago

hand-dot commented 5 months ago

https://github.com/pdfme/pdfme/assets/24843808/559ee9d9-9b60-4d5f-889c-91bfef362a65


TODO

vercel[bot] commented 5 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment | Name | Status | Preview | Comments | Updated (UTC) | | :--- | :----- | :------ | :------- | :------ | | **pdfme-playground** | ⬜️ Ignored ([Inspect](https://vercel.com/labelmake/pdfme-playground/8fg3W75ZFYcMu59EiyQxRtNm64Xz)) | [Visit Preview](https://pdfme-playground-git-add-readonly-schemas-labelmake.vercel.app) | | Dec 23, 2023 8:11am |
hand-dot commented 5 months ago

@peteward The build is failing in CI, but the implementation is already complete, so could you please review it?

I added a template named pet.json in packages/generator/tests/assets/templates/pet.json. This template is designed using a readOnly schema instead of basePdf.

https://github.com/pdfme/pdfme/assets/24843808/7d043ab0-784b-4d58-ad7e-445973cd608b

Since the design can be completed within pdfme, it should be possible to support schemas like dynamic tables, where the height changes depending on the data, in the future.

hand-dot commented 5 months ago

@peteward

Thank you for the insightful comment.

When I first considered the design for ReadOnly, I initially had it switchable via a checkbox in the DetailView. However, when creating Line, Rectangle, and Ellipse, these are always ReadOnly, so I separated them into different schemas as ReadOnly and non-ReadOnly, as it is now.

In fact, with some shapes and the newly added Text, Image, and SVG, it seems unlikely that users would often add ReadOnly schemas, right? Considering this, I think enforcing a ReadOnly setting might increase the workload when creating new plugins.

I'm designing pdfme to be as uncomplicated as possible. While I may not fully understand your use case, can't the implementation be done by extending the Text schema without using the ReadOnly schema?

Would you like to create the plugin your team desires after merging this PR? If there are clear specifications and goals documented, I will support as much as I can.

peteward commented 5 months ago

I will push up my branch now to show you what I am doing, it might make more sense then.

peteward commented 5 months ago

@hand-dot your argument for Read Only as different schema types makes sense 👍

see my WIP plugin code to understand what I am trying to achieve and see if there is any overlap with your data structure.

hand-dot commented 5 months ago

@peteward Okay I'm checking your new WIP PR.

Can i merge this PR? I do not intend to release this PR until it resolves the issues with your use case, even after merging. It's also possible to make readOnlyValue into content.

peteward commented 5 months ago

Of course!