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 DynamicTable Schema #332

Closed
2 of 6 tasks
hand-dot opened this issue Nov 25, 2023 · 20 comments
Closed
2 of 6 tasks

Add DynamicTable Schema #332

hand-dot opened this issue Nov 25, 2023 · 20 comments
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed idea @pdfme/schemas table

Comments

@hand-dot hand-dot added enhancement New feature or request @pdfme/schemas labels Nov 25, 2023
@hand-dot hand-dot changed the title Add Table Schema Add DynamicTable Schema Dec 1, 2023
@MP70
Copy link

MP70 commented Dec 29, 2023

I was reading your schemas/src in order to make modifications needed for my fork anyway and I think from a quick read there's very little actually needed to drop draw-table in if you're fine with it 'as is'. Schema wise, you can just have it pass through the table array and options object directly (minus the fonts and which it looks like you translate from strings) and just use it's types more or less directly, I think we are probably talking something in the order of 100-150 LOC.

However you might then have a case where you have slightly different features for a table cell vs normal 'pdfme' drawn text, for example draw table supports links, and might expose some slightly different syntax for styling text. In a sense it could be more ideal for it to 'call back' to your own standard drawText function, rather than having draw table do the drawing the text itself. That would also enable nested tables. If this route is desired I will need to make a few changes to draw-table but I'm open to it if you can ensure you're able to measure the height/width you need for a cell and draw the text (providing callbacks for both).

One thing to note is that we are measuring text heights/widths in slightly different ways, we can do some testing and see if your method produces better results. Looks like the same results to me, though I've not tested extensively.

For the UI, simply make use of the measuring functions that are exposed. This tells you the width of each column and the height of each row. You can call these separately to the actual 'draw table' function. UI component wise there may be some reusable things I can send you to give a head start, but I am currently not really allowing actual table editing in the drag drop designer other than positioning and reflecting styling/content changes made in a separate component. You might want to take a different approach, but this works best for me.

@hand-dot
Copy link
Collaborator Author

@MP70

Thank you.
Firstly, regarding the support for links, I hadn't planned to include it in V4, but if it's necessary, I can add it.
#319

The idea of performing drawing via callbacks is very interesting. This means that the table schema would be limited to calculating the layout and style of the table, and the content of the table would be drawn through callbacks, right?

As for measuring the height and width of text, I'm not yet sure which method to use, but generally, the text schema of pdfme, including the dynamic font size feature, works well. It functions correctly not only with Latin characters but also with Japanese and Chinese fonts. However, it's not perfect. There was a recent bug report titled 'Line breaks in RTL languages'.

What I want to clarify is what features this DynamicTable Schema will have. I want to define the requirements before writing any code. I also want to make sure that these features are not overly complex.

Pdfme aims to be a user-friendly PDF library. Therefore, I think it's necessary to discard overly specialized features. The code you've written was originally developed for in-house use and then extracted. Before integrating it into the core library of pdfme, I want to clarify what is necessary and what is not.

Is there a way to check what features the current pdf-lib-draw-table supports and what requirements it had?

@hand-dot hand-dot added help wanted Extra attention is needed idea labels Dec 31, 2023
@hand-dot hand-dot pinned this issue Jan 2, 2024
@MP70
Copy link

MP70 commented Jan 3, 2024

No worries. Apologies for the delay in response, it's been a busy New Year!

"The idea of performing drawing via callbacks is very interesting. This means that the table schema would be limited to calculating the layout and style of the table, and the content of the table would be drawn through callbacks, right?"

Yes. Schema-wise in draw table, it could be a type of cell content to add, similar to how we have customStyledText, or image/link. We might add something like customDrawnContent { MeasureCallback: void, DrawCallback: void}. Otherwise if there is no need to mix and match then it could take a single call back function in 'options' or in place of 'tabledata'. I'd need to have a think about it. On the other hand, draw table already exposes a way of drawing for all primitives except shapes, e.g.,

  • Text,
  • MultiLine Text (where you choose the line breaks),
  • Custom Styled Text,
  • Images,
  • Links,

which you could use today with no modification needed to the draw table library. So if those are acceptable then it's literally just a case of dropping it in.

"As for measuring the height and width of text, I'm not yet sure which method to use, but generally, the text schema of pdfme, including the dynamic font size feature, works well. It functions correctly not only with Latin characters but also with Japanese and Chinese fonts. However, it's not perfect. There was a recent bug report titled 'Line breaks in RTL languages.'"

Fair enough, draw table has only been tested in LTR Latin languages, but RTL and Japanese/Chinese fonts should, in theory, work. A corner case might be on word splitting.

"Pdfme aims to be a user-friendly PDF library. Therefore, I think it's necessary to discard overly specialized features. The code you've written was originally developed for in-house use and then extracted. Before integrating it into the core library of pdfme, I want to clarify what is necessary and what is not."

As it happens, this was actually written from the start as an open-source package. The general idea was that it should be simple to use for a basic use case but enable a decent amount of customization and overriding where the user needed. As such, all 'options' in draw table are exactly that - optional with defaults built in so you can choose to pass an empty options object if you like. It's up to you if you only want to expose a subset of draw table's features to your users. I would recommend exposing some bare minimum things like font, text size, has header, header background, border color, and so on, but it would be your choice. You can see this in the project readme.

@hand-dot
Copy link
Collaborator Author

hand-dot commented Jan 4, 2024

@MP70

Thanks for your reply.

I have put together some ideas on how to implement a table in the following article, so please refer to it if you have time.
Implementation Ideas for Dynamic Tables in pdfme

I'm not sure how much of the above ideas can be applied to draw table at this point, but it seems necessary to clarify that.
I will find time to simply incorporate into pdfme what is already achievable with draw table and give it a try.

I will check what kind of UX it will be and, if it looks good, I would like to implement it using draw table as it is.

I will update you if there are any changes.


I have a question.
Is it possible to add width to drawTable?
It is possible to calculate width using startX and pageMargin, but by adding width to drawTable, I think it will be simpler.
In pdfme, the size of the table is determined using the x, y coordinates and width, so it is necessary to be able to specify the width.

Also, it would be more user-friendly if there is an option like cellWidth in jsPDF-AutoTable to specify the width of columns.
↑ I found ColumnOptions.overrideWidths

@hand-dot
Copy link
Collaborator Author

hand-dot commented Jan 4, 2024

@MP70
The ability to set a background color for the 'td' element would be beneficial.

My.Movie.mp4

ref: #405

@MP70
Copy link

MP70 commented Jan 4, 2024

@MP70 The ability to set a background color for the 'td' element would be beneficial.

My.Movie.mp4
ref: #405

Totally agreed! I think from memory the only reason why I didn't was there was no clear place in the schema to pass it because it's not a table setting, nor a cell content :) It's trivial to add in once I work out the best place for the setting to go as it's already done for the header.

Would per row setting be fine or is per cell be better?

@hand-dot
Copy link
Collaborator Author

hand-dot commented Jan 4, 2024

@MP70
At this moment, Setting it up per row is fine.

@MP70
Copy link

MP70 commented Jan 4, 2024

@hand-dot

I have a question.
Is it possible to add width to drawTable?
It is possible to calculate width using startX and pageMargin, but by adding width to drawTable, I think it will be simpler.
In pdfme, the size of the table is determined using the x, y coordinates and width, so it is necessary to be able to specify the width.

Yeah I mean the margins were designed to be a more user friendly way of saying 'stop x' and 'stop y' but performing the calculations for the user so they didn't have to.

So for now you can just do page width - startx - table width to arrive at the page margin to pass to draw table. Maybe stopX and stopY would of been simpler after all, could probably add them in as an OR type with page margin, thanks so much for the feedback!

Also, it would be more user-friendly if there is an option like cellWidth in jsPDF-AutoTable to specify the width of columns.

This already exists in overrideWidths: number[] where each number in the array is the width of each column. If you're manually setting column widths then you must provide a width for every column. Is there something about this interface that doesn't work for you, anything I can improve?

@hand-dot
Copy link
Collaborator Author

hand-dot commented Jan 4, 2024

@MP70

This already exists in overrideWidths: number[] where each number in the array is the width of each column.
I missed it! Thank you.

Can i see the source code of https://pdf-lib-table-demo.vercel.app/?
I'd like to know how to achieve page break.

@noxify
Copy link

noxify commented Jan 4, 2024

@hand-dot could be this one: https://github.com/MP70/pdf-lib-draw-table

edit: wrong link - my bad, didnt checked the repo in detail, saw the mentioned link in the description and assumed this is the related repo 🙈

@hand-dot
Copy link
Collaborator Author

hand-dot commented Jan 4, 2024

@hand-dot could be this one: https://github.com/MP70/pdf-lib-draw-table

this is a source code of the pdf-lib-draw-table library, i'd like to know source code of demo app.
i couldn't find behavior of a page break from https://github.com/MP70/pdf-lib-draw-table.

@hand-dot hand-dot self-assigned this Jan 4, 2024
@MP70
Copy link

MP70 commented Jan 5, 2024

@hand-dot
Page Break
You're right in that we let users do the page breaking because then they can do it the way they like, rather than imposing 'the one way' but it does provide the tools to make it super simple, and I will be happy to share some examples with you. If we can find a common setting that most users want, without overcomplicating the library it could potentially be in scope for inclusion.

The demo does page breaking if a render fails due to overflowing table, so it is quite simplistic. However I would expect you'd want to know if you're going to overflow a page before you actually send off for rendering the table on a pdf because you might want to know for the designer?

Background Color
I have just popped this in the table options under options.row. I don't really love that home for it, but it gets you what you need. It's an array of color or undefined.
so this

      table.options.row = {
        ...table.options.row,
        backgroundColors: [table.options.header.backgroundColor,undefined,table.options.header.backgroundColor],
      }

would set row 1 to have a color, row 2 no color, and row 3 a color.

NB: This setting is only for non header rows, so index 0 color will be the color applied to row 0 in a no header table, and row 1 in a header table.

@MP70
Copy link

MP70 commented Jan 5, 2024

Thats released as 0.0.75, you'll need to upgrade. Here is the interface for backgroundColors https://mp70.github.io/pdf-lib-draw-table/interfaces/RowOptions.html

@bgrand-ch
Copy link

Hello, thanks for PDFME, I can't wait for this future feature!

@wiryonolau
Copy link

wiryonolau commented Feb 18, 2024

Maybe just use simple approach instead of pushing it automatically, maybe let user define max row for each table in a page. something like limit offset in database.

We might need to create repeat for many page but I think it's more straight forward.
Then we just need to check total row and count which page need to be print.

@hand-dot
Copy link
Collaborator Author

Hey @wiryonolau
Interesting. Can you elaborate on this?

@wiryonolau
Copy link

wiryonolau commented Feb 18, 2024

I mean let the user handle all the row count, cell width , row height design adjustment.
If it break the page they will need to readjust the design.

For example :
page 1 - table only 5 row
page 2 - table only 10 row
page 3 - table only 10 row

we then can create 3 version , a template that only use page 1, a template that only use page 1 and 2 and so on.

When providing a data that has 12 row, it means it require template with 2 page.
On loading the data first page will only take first 5 row, second page the rest of the row.

It's a bit stupid but I think it will works for the user.
I would love to have the auto page break when adding new row like excel, but I think it's over complicated for a printout report.

@hand-dot hand-dot added the table label Feb 24, 2024
@hand-dot
Copy link
Collaborator Author

Hey @wiryonolau,
Thank you for the proposal.
I am considering offering an implementation for auto page break.

I have already made some progress: https://add-dynamic-table-schema-332.vercel.app/
It's complex and there are bugs, but I want to proceed in this direction.

hand-dot added a commit that referenced this issue Feb 24, 2024
* TMP

* [tmp] add some comment

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* Update snapshot

* TMP

* Add deploy-table script to package.json

* add new template for playground

* bug fix for form

* fix cell editing bug

* Fix Adding rows doesn't change the overall height of the table

* fix padding problem

* Fix build error

* Fix bug

* Minor fix

* Fix New lines not reflecting correctly

* minor fix

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* Minor fix

* TMP

* TMP

* TMP

* TMP

* TMP

* Change tableStyles def

* add i18n

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* small bugfix

* TMP

* TMP

* FIx some TODO

* Remove japanese comment

* Minor fix

* Fix infinity loom for form

* fix save inputs bug

* fix window resize bug

* add skip for failing test and update snapshot

* Minor fix

* add presets for playground

* Minor fix
hand-dot added a commit that referenced this issue Feb 24, 2024
* TMP

* [tmp] add some comment

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* Update snapshot

* TMP

* Add deploy-table script to package.json

* add new template for playground

* bug fix for form

* fix cell editing bug

* Fix Adding rows doesn't change the overall height of the table

* fix padding problem

* Fix build error

* Fix bug

* Minor fix

* Fix New lines not reflecting correctly

* minor fix

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* Minor fix

* TMP

* TMP

* TMP

* TMP

* TMP

* Change tableStyles def

* add i18n

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* TMP

* small bugfix

* TMP

* TMP

* FIx some TODO

* Remove japanese comment

* Minor fix

* Fix infinity loom for form

* fix save inputs bug

* fix window resize bug

* add skip for failing test and update snapshot

* Minor fix

* add presets for playground

* Minor fix
@hlin-neo4j
Copy link
Contributor

One thing I wanted to add in case it hasn't been tracked, but I'm noticing that the placement of the tables in the pdf from Generate PDF sometimes differs from what's on the Designer. This is whenever I add the property to the table schema "readOnly: true", then call "designer.updateTemplate(template)"

hand-dot added a commit that referenced this issue Apr 11, 2024
* Reconsider the internal data types and the template types (#389)

* Change SchemaForUI's data and readOnlyValue to content

* Fix bug

* Remove sampledata and instead always use content. & Use content regardless of readOnly or not. & Eliminate defaultValue and replace it with always using content.

* Change website template

* Fix placeholder bug

* Change generator test template

* Change content to optional

* Move getInputFromTemplate to common

* Remove columns

* Fix test

* move dynamictable.excalidraw

* remove idea dir

* New basePdf type and Support adding new pages to template (#394)

* Minor fix

* IMPL Support adding new pages to template #111

* Fix test

* Minor fix

* IMPL padding behavior

* Update snapshot

* Minor fix

* Update snapshot

* add i18n

* remove option from BlankPdf.padding

* Minor fix

* Minor fix

* format

* Add changeSchemas unit test (#403)

* Minor fix

* Add a version number to pdfme template from V4 onwards (#404)

* Impl

* Minor fix

* Padding move width (#407)

* Refactor position and size handling in helper.ts

* Fix bug

* Add DynamicTable Schema #332 (#408)

* [tmp] add some comment

* Update snapshot

* Add deploy-table script to package.json

* add new template for playground

* bug fix for form

* fix cell editing bug

* Fix Adding rows doesn't change the overall height of the table

* fix padding problem

* Fix build error

* Fix bug

* Minor fix

* Fix New lines not reflecting correctly

* minor fix

* Minor fix

* Change tableStyles def

* add i18n

* small bugfix

* FIx some TODO

* Remove japanese comment

* Minor fix

* Fix infinity loom for form

* fix save inputs bug

* fix window resize bug

* add skip for failing test and update snapshot

* Minor fix

* add presets for playground

* Minor fix

* Reconsider the internal data types and the template types (#389)

* Change SchemaForUI's data and readOnlyValue to content

* Fix bug

* Remove sampledata and instead always use content. & Use content regardless of readOnly or not. & Eliminate defaultValue and replace it with always using content.

* Change website template

* Fix placeholder bug

* Change generator test template

* Change content to optional

* Move getInputFromTemplate to common

* Remove columns

* Fix test

* move dynamictable.excalidraw

* remove idea dir

* New basePdf type and Support adding new pages to template (#394)

* Minor fix

* IMPL Support adding new pages to template #111

* Fix test

* Minor fix

* IMPL padding behavior

* Update snapshot

* Minor fix

* Update snapshot

* add i18n

* remove option from BlankPdf.padding

* Minor fix

* Minor fix

* format

* Add changeSchemas unit test (#403)

* Minor fix

* Add a version number to pdfme template from V4 onwards (#404)

* Impl

* Minor fix

* Padding move width (#407)

* Refactor position and size handling in helper.ts

* Fix bug

* Add DynamicTable Schema #332 (#408)

* [tmp] add some comment

* Update snapshot

* Add deploy-table script to package.json

* add new template for playground

* bug fix for form

* fix cell editing bug

* Fix Adding rows doesn't change the overall height of the table

* fix padding problem

* Fix build error

* Fix bug

* Minor fix

* Fix New lines not reflecting correctly

* minor fix

* Minor fix

* Change tableStyles def

* add i18n

* small bugfix

* FIx some TODO

* Remove japanese comment

* Minor fix

* Fix infinity loom for form

* fix save inputs bug

* fix window resize bug

* add skip for failing test and update snapshot

* Minor fix

* add presets for playground

* Minor fix

* Minor fix

* Update imports and fix font rendering

* Add a Left Sidebar for Placing Schemas #400 (#452)

* Remove original 'Add new field' Button

* add icon

* FIx drag position bug

* Minor fix

* Fix sidebar position

* Update snapshot

* Minor fix

* Update packages/ui/src/components/Designer/index.tsx

Co-authored-by: Peter Ward <pete@pennyblack.io>

---------

Co-authored-by: Peter Ward <pete@pennyblack.io>

* Fix test

* Improve left sidebar icon drop placement accuracy (#454)

* Fix Spanish translations for v4 (#463)

* Fix #431

* V4 (#467)

* feat: add french language

* feat: relecture

* feat: add french language

---------

Co-authored-by: regis <regis>

* Add French language option to playground

* rename table export name to tableBeta

---------

Co-authored-by: Peter Ward <pete@pennyblack.io>
Co-authored-by: Iker Diez <32014358+ikerd@users.noreply.github.com>
Co-authored-by: Régis <regis.charnace@leandco.fr>
@hand-dot hand-dot mentioned this issue Apr 11, 2024
Merged
@hand-dot
Copy link
Collaborator Author

The DynamicTable Schema has been released as Beta in pdfme V4.

For more details, please refer to the following documentation:
https://pdfme.com/docs/tables

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed idea @pdfme/schemas table
Projects
No open projects
Status: Done
Development

No branches or pull requests

6 participants