chunyenHuang / hummusRecipe

A powerful PDF tool for NodeJS based on HummusJS.
https://hummus-recipe.s3.amazonaws.com/docs/Recipe.html
MIT License
339 stars 91 forks source link

Match table data columns to column options #209

Open phynweb opened 3 years ago

phynweb commented 3 years ago

@shaehn I'd like to suggest a small change to the table.js code, specifically in the code block below

`if (!options.columns || options.columns.length !== fields.length) {
        order = fields; // all fields emitted, columns in order of first record
    } else {
        // columns in order found in options
        for (const column of options.columns) {
            order.push(column.name);
        }
    }`

The impact of || options.columns.length !== fields.length is that if i have an object with say 8 keys, but I only want to create a table with 5 columns (i.e. 5 keys), I have to create a separate objectwhere i've extracted just the 5 keys I'd like to display. If i stick with my original object, I get 3 more columns that are unformatted since I haven't passed any column options for them.

For my use-case, it is quite trivial to create another object. However, I am toying around with adding hyperlinks using a combination of hummusRecipe and hummusJS directly. Hence, I need the data to contain other data not shown in the table i.e. the url. I can also envision a need for extraneous data that shouldn't be displayed passed to the table in events where anything data driven is being done in a table using data that isn't explicitly displayed.

To accomodate this, we would need to change -

if (!options.columns || options.columns.length !== fields.length)

to

if (!options.columns || !options.columns.length)

Please let me know if there's some downstream impact i'm missing.

shaehn commented 3 years ago

@phynweb, Hmm... what am I missing here, because I am not envisioning what the problem is? I think you should be able to have a table with any number of columns and you emit only the ones you want by specifying the column name in options.columns. That is the driving force behind the original design. You either get all columns by default when you don't specify options.columns, or you direct traffic by specifying what columns are desired and the order to display them. You don't have to change the given table 'contents'.

phynweb commented 3 years ago

The way the code is written right now, even if you specify the columns you want, if they are less than all the fields in the data, it defaults to all the columns.

table.js [Line 105] - let fields = Object.keys(contents[0]); i.e. get all keys in the first row...using my previous example, lets say this is equals to 8. If my column specification only defines 5 fields, once we get to lines 115 i.e. if (!options.columns || options.columns.length !== fields.length) the table would default to line 116 order = fields; // all fields emitted, columns in order of first record because options.columns.length (5) is not equal to fields.length (8)

Please let me know if i'm doing this wrong, because up till now, I've had to pare down my data fields to those specified in column options because they kept showing up with native formatting (blue text)

shaehn commented 3 years ago

Ok, that makes sense now, but to protect the code from going off deep end, what if you change it to be

if (!options.columns || options.columns.length > fields.length)

This should protect the output to be only as much as in the table.

phynweb commented 3 years ago

Sounds good to me.