-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Feature/table cell vertical alignment #2436
base: master
Are you sure you want to change the base?
Feature/table cell vertical alignment #2436
Conversation
… image, canvas, qr, svg
This reverts commit a0781c5.
Hi, |
…b.com/SirFull/pdfmake into feature/table_cell_vertical_alignment
Agreed. Eager for this feature. Thanks for your work! |
Can't wait for this to be merged |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need all the begin/end - Vertical alignment
comments everywhere? Git already shows a nice diff.
Alos why do we need the __
prefix everywhere? Rest of this project seems to use "normal" names.
src/LayoutBuilder.js
Outdated
// begin - Vertical alignment | ||
const lastNode = this.__nodesHierarchy.pop(); | ||
lastNode.__contentHeight = Math.max(...columns.map(c => c.__contentHeight)); | ||
this.__nodesHierarchy[this.__nodesHierarchy.length - 1].__contentHeight += lastNode.__contentHeight; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is a missing check for this.__nodesHierarchy.length > 0
src/LayoutBuilder.js
Outdated
@@ -613,6 +648,10 @@ class LayoutBuilder { | |||
this.writer.removeListener('lineAdded', addMarkerToFirstLeaf); | |||
|
|||
this.writer.context().addMargin(-gapSize.width); | |||
// begin - Vertical alignment | |||
const lastNode = this.__nodesHierarchy.pop(); | |||
this.__nodesHierarchy[this.__nodesHierarchy.length - 1].__contentHeight += lastNode.__contentHeight; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
another missing check for this.__nodesHierarchy.length > 0
src/TableProcessor.js
Outdated
cellHeight = table.__rowsHeight[rowIndex].height; | ||
} | ||
if (cellHeight) { | ||
const pageItems = writer._context.pages.flatMap(x => x.items); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note for people porting this to the 0.2 branch: This line needs to be
const pageItems = writer.writer.context.pages.flatMap(x => x.items);
|
are there news regarding this? |
Any estimate on when this can be merged? I think this feature will greatly improve pdfmake tables! |
Guys please, we've been asking for this feature since 2014 ❤️ |
It would be so awesome to have this merged in, plsplspls |
use paddingTop in processTableVerticalAlignment
Is there a problem with this PR? So we can update it and finally merge it? |
@Kran67 have you addressed the changes so this can get merged? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added what I believe are the suggested changes so we can rebase inside github if desired.
src/LayoutBuilder.js
Outdated
// begin - Vertical alignment | ||
const lastNode = this.__nodesHierarchy.pop(); | ||
lastNode.__contentHeight = Math.max(...columns.map(c => c.__contentHeight)); | ||
this.__nodesHierarchy[this.__nodesHierarchy.length - 1].__contentHeight += lastNode.__contentHeight; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this.__nodesHierarchy[this.__nodesHierarchy.length - 1].__contentHeight += lastNode.__contentHeight; | |
this.__nodesHierarchy.length > 0 && (this.__nodesHierarchy[this.__nodesHierarchy.length - 1].__contentHeight += lastNode.__contentHeight); |
Suggested check
src/LayoutBuilder.js
Outdated
@@ -613,6 +648,10 @@ class LayoutBuilder { | |||
this.writer.removeListener('lineAdded', addMarkerToFirstLeaf); | |||
|
|||
this.writer.context().addMargin(-gapSize.width); | |||
// begin - Vertical alignment | |||
const lastNode = this.__nodesHierarchy.pop(); | |||
this.__nodesHierarchy[this.__nodesHierarchy.length - 1].__contentHeight += lastNode.__contentHeight; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this.__nodesHierarchy[this.__nodesHierarchy.length - 1].__contentHeight += lastNode.__contentHeight; | |
this.__nodesHierarchy.length > 0 && (this.__nodesHierarchy[this.__nodesHierarchy.length - 1].__contentHeight += lastNode.__contentHeight); |
Suggested check
Would love to see this feature merged soon. |
update pls. |
I'm in need of this feature, could this be merged and released? |
Would be great if this is merged 👍 |
I prefer to install this library from the SirFull's fork rather than wait for this PR to be merged. Fortunately, package managers such as npm can do it directly from github from the specific branch: |
unable to install
|
Sorry, I'm new to the web development and the stuff around it, so I have no idea where the problem is. I installed this library form the specific commit right now (2nd example) and everything seems to be okay. This line |
Hello, @Kran67 and me are working together, last week i will try to merge last version and ill add recommended suggestions from here. |
I updated the fork with the latest updates and suggestions. I also added a way to stretch the table height according to the parent. To do this, you need to set a line height to "*", the generation will then be done twice to know the available height of the parent and modify the line height accordingly. |
Table cell vertical alignment - for stack, column, list, text, image, qr, svg