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

Match table data columns to column options #209

Open
phynweb opened this issue Mar 1, 2021 · 4 comments
Open

Match table data columns to column options #209

phynweb opened this issue Mar 1, 2021 · 4 comments

Comments

@phynweb
Copy link
Contributor

phynweb commented Mar 1, 2021

@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
Copy link
Collaborator

shaehn commented Mar 1, 2021

@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
Copy link
Contributor Author

phynweb commented Mar 1, 2021

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
Copy link
Collaborator

shaehn commented Mar 1, 2021

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
Copy link
Contributor Author

phynweb commented Mar 2, 2021

Sounds good to me.

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

No branches or pull requests

2 participants