Skip to content

Add column_alignments to addSheetFromData#54

Open
samal-rasmussen wants to merge 1 commit intoNeovici:masterfrom
samal-rasmussen:master
Open

Add column_alignments to addSheetFromData#54
samal-rasmussen wants to merge 1 commit intoNeovici:masterfrom
samal-rasmussen:master

Conversation

@samal-rasmussen
Copy link

Thank you very much for this absolute fantastic little lib. Only additional thing I needed was to do some column alignment, so I implemented it. Feel free to ignore this pr if you wan to keep your lib teeny tiny. Just sharing the pr so others can see it.

Copy link
Collaborator

@cristinecula cristinecula left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @samal-rasmussen!

Thanks a lot for your contribution! We're really glad you found value in our library! Please excuse the late response. 🙏

I think that this would be a great enhancement. I only have a couple of suggestions/questions:

return `<c s="${ dateStyle }"><v>${ this.dateToExcelDate(cell) }</v></c>`;
}
return `<c t="inlineStr"${ style }><is><t>${ this.escapeXml(cell.toString()) }</t></is></c>`;
return `<c t="inlineStr"${ style }${ alignment_style }><is><t>${ this.escapeXml(cell.toString()) }</t></is></c>`;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could potentially lead to having two s attributes on a cell: s="1" s="4"
Did you test if that won't result in an invalid document?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me. No error reports from users yet 🤷‍♂️ But I really have no clue what I'm doing and I don't trust that I'm doing anything the right way here - only that it works for me.

* @return {NullXlsx} Returns itself for method chaining
*/
addSheetFromData(data, name) {
addSheetFromData(data, name, column_alignments) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to send in column_alignments as part of an options object, to allow room for any additional options.

Suggested change
addSheetFromData(data, name, column_alignments) {
addSheetFromData(data, name, options) {

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