Skip to content

added new table features: evenColumns, text color, background color#42

Merged
ole1986 merged 13 commits intorospdf:masterfrom
Manuzza:feature/table-enhancements
Nov 16, 2016
Merged

added new table features: evenColumns, text color, background color#42
ole1986 merged 13 commits intorospdf:masterfrom
Manuzza:feature/table-enhancements

Conversation

@Manuzza
Copy link
Copy Markdown
Collaborator

@Manuzza Manuzza commented Nov 12, 2016

New table option: "evenColumns"

default value = 0 (it does nothing)

If evenColumns = 1 then instead of calculating the width based on the size of the content it will spread the columns evenly (each column will be the same width). Note that it will only evenly distribute columns that do NOT have a specified width. For example, columns that have a manually specified width will continue to use that width.

If evenColumns = 2 then it will only evenly balance the columns if the following criteria is met:

  1. The first width calculations were wider than the available space.
  2. One of the recalculated columns is very narrow.

This solves the problem that occurs when a column is very narrow and the table generator gets stuck in a loop adding page after page trying to make the content fit.

New cell option for specifying the Text and Background color:

Every single cell in a table can now have its own specific background color AND text color. For any cell in the data array you want to change the colour, simply add the fields [columnName]Fill and [columnName]Text with a color array.

For example if we enhance the example in the manual (additions in bold):

$data = array(
array('num'=>1,'name'=>'gandalf','type'=>'wizard','typeFill'=>array(0,1,0))
,array('num'=>2,'name'=>'bilbo','type'=>'hobbit','typeFill'=>array(0.588235294118, 0.407843137255, 0))
,array('num'=>3,'name'=>'frodo','type'=>'hobbit','typeFill'=>array(0.588235294118, 0.407843137255, 0))
,array('num'=>4,'name'=>'saruman','type'=>'bad dude','typeFill'=>array(1,0,0),'typeText'=>array(1,1,1,))
,array('num'=>5,'name'=>'sauron','type'=>'really bad dude','typeFill'=>array(0,0,0),'typeText'=>array(1,1,1,))
);

The cell containing the word:
"wizard" will have a green background.
"hobbit" will have a brown background.
"bad dude" will have a red background with white text.
"really bad dude" will have a black background with white text.

Background colors of a cell will be selected using the following precedence:

  1. A specified color for the cell.
  2. A specified color for the column.
  3. The shadeCol or shadeCol2 (depending on what "shaded" is set to).

@ole1986
Copy link
Copy Markdown
Collaborator

ole1986 commented Nov 13, 2016

@Manuzza
there are several issue I have with this PR:

Many PHP Notices

Notice: Undefined index: evenColumns in /Users/ole/Documents/pdf-php/src/Cezpdf.php on line 1171
Notice: Undefined index: pText in /Users/ole/Documents/pdf-php/src/Cezpdf.php on line 1514
Notice: Undefined index: pFill in /Users/ole/Documents/pdf-php/src/Cezpdf.php on line 1515
Notice: Undefined variable: COLOR_ARRAY_SENT in /Users/ole/Documents/pdf-php/src/Cezpdf.php on line 1522
Notice: Undefined variable: decender in /Users/ole/Documents/pdf-php/src/Cezpdf.php on line 1539
Notice: Undefined index: bgcolor in /Users/ole/Documents/pdf-php/src/Cezpdf.php on line 1544
Notice: Undefined variable: option in /Users/ole/Documents/pdf-php/src/Cezpdf.php on line 1560
Notice: Undefined variable: option in /Users/ole/Documents/pdf-php/src/Cezpdf.php on line 1560
Notice: Undefined variable: option in /Users/ole/Documents/pdf-php/src/Cezpdf.php on line 1560

I had to use error_reporting(E_ALL & ~E_NOTICE);

Using option "evenColumn => 1" causes th table to move out of the docuement

evencolumn_1

[columnText] and [columnFill] does not seem to work

I have added an example file "examples/table-enhancements.php".
Either I am doing it wrong (then please correct the example file) or it does not seem to work.

@ole1986
Copy link
Copy Markdown
Collaborator

ole1986 commented Nov 13, 2016

In addition to my previous message I would color the text with the custom callback:

<c:color:r,g,b>Any colored Text</c:color>

The cell filling of course is a good idea

@Manuzza
Copy link
Copy Markdown
Collaborator Author

Manuzza commented Nov 13, 2016

Hi,

Code now complies with E_ALL...

Most of our tables are left aligned so evenTables had bugs related to non-left aligned tables... :-) Changed the code so that IF the table maxWidth is NOT set, evenColumns will use the full width of the page (between the pageMargin values, of course). Therefore it effectively ignores xPos and xOrientation. If a maxWidth is set, the table will be positioned according to xPos and xOrientation (tables can still go off the screen if a user sets silly parameters - that is on the user). evenColumns is a recent feature addition by me that was probably not quite ready.

The cell colors, on the other hand, was done 5 or so years ago, BEFORE the <c:color> tag. I have converted our application to this method BUT maintained this as an extra setting "[colName]Text". This is superior to wrapping the whole sell in a <c:color> tag because as soon as the table cell wraps to the next line the color of the text is lost. <c:color> is fine for short text cells only (see the table examples - two big columns side-by-side with one using a <c:color> tag and the other applying a color to that column).

It is possible that evenColumns still has bugs as there are MANY variations to test...

@ole1986
Copy link
Copy Markdown
Collaborator

ole1986 commented Nov 13, 2016

Well done,

I am going to check the <c:color:r,g,b> - tag to support new lines in cells as well.
So, we will only implement the [columnName]Fill?

Regarding the option evenColumns => 1,2 please make sure it will also work for centered and right aligned tables...

@Manuzza
Copy link
Copy Markdown
Collaborator Author

Manuzza commented Nov 13, 2016

We use a wrapper class to build our tables so if you change the [columnName]Text method to <c:color> we can adjust all of our tables in on place. With regards to <c:color> - I have set up table-examples.php so it fails. The text in the middle column is slightly pink so you can see it on the last column (on white). When a colored column follows another colored column it fails. Also note that you need to check that the color persists when the table cell spans multiple pages (which is what the attached sample does).

I have left the [columnFill]Text in the code base but commented it out - therefore it is there if we need it back.

@Manuzza
Copy link
Copy Markdown
Collaborator Author

Manuzza commented Nov 14, 2016

Regarding evenColumns... it does work with centred and right tables...

If you do NOT give a maxWidth the maxWidth is effectively the space between the margins and so the table spreads out across the available space. Since the table uses everything between the left and right margins the alignment is irrelevant as the table will look the same in all cases.

When maxWidth is specified the even spread is across that width. For example, if the column has a maxWidth of 15cm and there are four columns, each column will be 5cm. The table will be positioned left, right or centre as specified...

Using evenColumns = 2 with no maxWidth means that is a centred table is narrow enough, it will be centred on the page using the space that it needs and no more. Each column will have its own width. But a table with a lot of content means it spreads out between the margins.

Remember, you can still manually specify the with of any of the columns. In the example table you can specify a narrow width for the "No" column and it is guaranteed to use that width, not matter what other settings are. Therefore you can force a column to be narrower or wider, depending on the content that is in them...

@ole1986
Copy link
Copy Markdown
Collaborator

ole1986 commented Nov 14, 2016

Using custom tags in table cells can be a design error and properly makes more sense to use your solution with the [columnName]Text key. But let take a closer look into this first.

Also I would prefer to use [columnName]Color instead of [columnName]Text


Also noted that the evenColumns option does not causes the messy table
It seems to be another bug where width is not defined by default (Issue #43)

@Manuzza
Copy link
Copy Markdown
Collaborator Author

Manuzza commented Nov 14, 2016

I am happy with renaming those params - we use a wrapper class that sits above Cezpdf to manage tables (so building correct arrays is guaranteed). I can update every single table we generate by changing a single function... :-)

I saw the other solution for coloring text and thought it looked messy to implement - that is why i left the code there and only commented it out... :-)

Os there anything else I need to do on this for now? Do you want me to put the code back and change the variable name?

Is Issue #43 in your hands for now?

@ole1986
Copy link
Copy Markdown
Collaborator

ole1986 commented Nov 14, 2016

I have fixed the issue #43
So there is no need for option evenColumns = 2 in my opinion


Also noted another issue which is (i think) related to your [columnName]Fill option.
It actually overwrites the column and heading backgrounds (if set) and also overwrites the default in some situations

Heading background is usually set through: 'shadeHeadingCol' => array(r,g,b)
Column background can be used in:

'showBgCol' => 1,
'cols'=> array(
                    '[columnName]'=>array('bgcolor'=>array(0.9,0.9,0.7))
)

Examples located in: examples/tables.php

@ole1986
Copy link
Copy Markdown
Collaborator

ole1986 commented Nov 14, 2016

I know its pretty hard to customize even a single line in Cpdf without affecting other stuff.
Thats why I really want to have the Cpdf version 0.13 running - but that is another story.

Just want to let you know I am really happy that you are taking part of this project -
I am very optimistic we will merge it into master branch soon.

Thank you

@Manuzza
Copy link
Copy Markdown
Collaborator Author

Manuzza commented Nov 14, 2016

I have pushed with [columnName]Text restored but renamed to [columnName]Color...

The ability to color a column was not being interfered with by the [columnName]Fill - instead I was looking in the wrong damn place for fetching the column color (array path fixed).

I could not see an issue with the column headers...

The table-enhancements.php now has:

  • column headers are light brown
  • alternate row shading of two different light grey shades (not 50 shades, just 2)
  • the first column is assigned the color yellow
  • every cell in the second column has an assigned color

Therefore only the third column is visibly alternately shaded...

@Manuzza
Copy link
Copy Markdown
Collaborator Author

Manuzza commented Nov 14, 2016

Regarding evenColumns = 2:

We have always had intermittent problems with tables not rendering properly. We can have dozen of tables in an output and some of those have 8-10 columns in a table that can span 50 or so pages. Every now and again one column in one row that is usually no more that 10 characters can all of a sudden have 50 characters.

In the past when the generation crashes (insufficient RAM) the solution is to manually go comment out every table until you find the culprit and mess with it until you get it working again.

The evenColumns 2 is a catch-all that stops this from occurring on odd occasions. Using even columns can look ugly but at least the output is still guaranteed to work. We can then manually narrow or widen specific columns to improve the appearance as the need arises...

@ole1986
Copy link
Copy Markdown
Collaborator

ole1986 commented Nov 14, 2016

found the problem in line 1387 when 'shaded' => 1 every row becomes the shaded which is incorrect according to the documentation:

'shaded'=> 0,1,2,3 default is 1 (1->**alternate lines are shaded**, 0->no shading, 2-> both shaded, second uses shadeCol2)

So I fixed it

@ole1986
Copy link
Copy Markdown
Collaborator

ole1986 commented Nov 15, 2016

@Manuzza are you OK with these changes or do you want to get your evenColumns = 2 implemented.

IMO we should not work around the error they way you did but instead we should correct (set minimum colum width ) before your code lines.

What do you think?

@Manuzza
Copy link
Copy Markdown
Collaborator Author

Manuzza commented Nov 15, 2016

evenColumns 2 remains a catch all that only kicks in under a narrow range of parameters. If the column logic is improved it will trigger less which is fine but a table width calculation failure stops an entire PDF being generated.

Because our tables can be huge and unpredictable I would have it turned on always, but I understand that not everyone would want that.

We can also tweak the min width that is used to determine if it should run (was set to 40 units).

So yes I would like to put it back if that is ok...

@ole1986
Copy link
Copy Markdown
Collaborator

ole1986 commented Nov 16, 2016

@Manuzza can you provide me with an example where you had issues with the table generation?

@Manuzza
Copy link
Copy Markdown
Collaborator Author

Manuzza commented Nov 16, 2016

I cannot, as the examples contain proprietary client information.

What happens as we understand it is this:

One of the auto-columns is calculated to be too narrow for a single character. ezTable tries to put content in the cell and of course fails. ezTable therefore adds a page and fails again. It does this in a loop until the PHP time limit is reached OR the available memory limit is reached (whichever comes first).

As I said this is for extremely large tables (both width and height). When we get the issue we either edit the longest content to shorten it or manually force widths on the table. Finding WHICH table in a document can be quite time consuming.

These errors in production are still on the very old code base and they are quite rare. But when they do happen everything stops and it is a major burden. We generate hundreds (if not thousands) of documents a day.

When I tested the new code on one of our longest documents the table error occurred in numerous places (this was before the bug fixes you have put in place). evenColumns was my response to this.

@ole1986
Copy link
Copy Markdown
Collaborator

ole1986 commented Nov 16, 2016

I have optimized it a bit, so it should not exceed the max execution time so quickly.
First I would keep it that way..

But also issue #44 covers this case (incl. example file)
we can talk about it in issue #44

Thank you @Manuzza for your support

@ole1986 ole1986 merged commit db5f3f1 into rospdf:master Nov 16, 2016
@Manuzza
Copy link
Copy Markdown
Collaborator Author

Manuzza commented Nov 16, 2016

Hi,

Yet to test the merged but 2 comments...

It may be worth mentioning in the documentation that evenColumns does not touch any columns that have a specified width, so the user still has a measure of control over the layout.

Also, the table on page 8 of the manual looks broken somehow...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants