Row groups #723

Closed
Danwakeem wants to merge 0 commits from master into master
Danwakeem commented 2017-07-09 19:03:17 +00:00 (Migrated from github.com)

Added a groupLevel parameter to the row array that allow you to group rows together.

type RowInfo = {
	/* visibility */
	hidden?: boolean; // if true, the row is hidden

	/* row height is specified in one of the following ways: */
	hpx?:    number;  // height in screen pixels
	hpt?:    number;  // height in points

        /* Group rows together when generating a spread sheet */
        groupLevel: number; // level that the row belongs to
};

I tested this feature by recreating the the writexlsx demo and adding this to the !rows property.

wb.Sheets[ws_name]['!rows'] = [{groupLevel: 2},{groupLevel: 2},{groupLevel: 1}];

Note: I had to modify the dist phony in the make file. When I installed uglifyjs globally it looks like it no longer supported the --support-ie8 flag and it was replaced with the --ie8 flag. I also had to add a filename= to the --source-map flag. Not sure if this was needed for a pull request so feel free to ignore the build and makefile changes.

Added a `groupLevel` parameter to the row array that allow you to group rows together. ```typescript type RowInfo = { /* visibility */ hidden?: boolean; // if true, the row is hidden /* row height is specified in one of the following ways: */ hpx?: number; // height in screen pixels hpt?: number; // height in points /* Group rows together when generating a spread sheet */ groupLevel: number; // level that the row belongs to }; ``` I tested this feature by recreating the the [writexlsx demo](http://sheetjs.com/demos/writexlsx.html) and adding this to the `!rows` property. ```typescript wb.Sheets[ws_name]['!rows'] = [{groupLevel: 2},{groupLevel: 2},{groupLevel: 1}]; ``` _Note: I had to modify the dist phony in the make file. When I installed uglifyjs globally it looks like it no longer supported the `--support-ie8` flag and it was replaced with the `--ie8` flag. I also had to add a `filename=` to the `--source-map` flag. Not sure if this was needed for a pull request so feel free to ignore the build and makefile changes._
Danwakeem commented 2017-07-09 19:36:38 +00:00 (Migrated from github.com)

Sorry, I can't see what the issue is with the code climate check because I keep getting a 504 error.

Sorry, I can't see what the issue is with the code climate check because I keep getting a 504 error.
SheetJSDev commented 2017-07-09 20:15:38 +00:00 (Migrated from github.com)

@Danwakeem Thanks for looking into this! I will try to spend some time later today digging into the corner cases, but the representation looks good.

Later on we'll want to have support for the other formats as well. Here's my initial survey of the landscape:

XLSX

Is the default row outline level 1 or 0? ECMA-376 is somewhat unclear: outlinelevel.pdf

ECMA-376 section 18.3.1.13 covers the col xml node, and it says

Outline level of affected column(s). Range is 0 to 7.

ECMA-376 section 18.3.1.73 covers the row xml node, and it says

Outlining level of the row, when outlining is on.

XLS/XLSB

The equivalent XLSB field is 2.4.718 BrtRowHdr and the XLSB spec shows how the outline level is stored:

iOutLevel (3 bits): An unsigned integer that specifies the outline level for this row.

(which is consistent with 0-7 outline level)

The equivalent XLS field is 2.4.221 Row and the XLS spec uses the same representation as XLS

Other Formats

ODS/FODS use nested table:table-row-group tags to indicate outline level

SpreadsheetML and other formats have no support for grouping

Tests

In addition to manual verification, we'll also need to verify that round-tripping preserves the properties. Easiest way is to make a test file exploring corner cases and save in various formats.

https://github.com/SheetJS/js-xlsx/blob/master/test.js#L1403 is the relevant part of the test suite for row properties (starting from a blank slate)

https://github.com/SheetJS/js-xlsx/blob/master/test.js#L902 starts from the row_height test files. https://github.com/SheetJS/test_files/ search for row_height and you'll see:

row_height.biff5
row_height.slk
row_height.xls
row_height.xlsb
row_height.xlsx
row_height.xml

Based on quick tests, I suspect only the XLSX, XLSB, XLS, and BIFF5 (save as "Excel 5.0/95 Workbook") will be needed.

Misc

Don't worry about the code climate check -- we just removed it :)

Can you unwind the changes to the dist files? those are updated when we cut releases.

Note: We ended up having to fork the minifier to support extendscript. See https://github.com/mishoo/UglifyJS2/issues/1930 for the relevant issue.
https://www.npmjs.com/package/@sheetjs/uglify-js is our fork.

@Danwakeem Thanks for looking into this! I will try to spend some time later today digging into the corner cases, but the representation looks good. Later on we'll want to have support for the other formats as well. Here's my initial survey of the landscape: ### XLSX Is the default row outline level 1 or 0? ECMA-376 is somewhat unclear: [outlinelevel.pdf](https://github.com/SheetJS/js-xlsx/files/1134004/outlinelevel.pdf) ECMA-376 section 18.3.1.13 covers the col xml node, and it says > Outline level of affected column(s). Range is 0 to 7. ECMA-376 section 18.3.1.73 covers the row xml node, and it says > Outlining level of the row, when outlining is on. ### XLS/XLSB The equivalent XLSB field is 2.4.718 `BrtRowHdr` and the [XLSB spec](https://msdn.microsoft.com/en-us/library/dd773045.aspx) shows how the outline level is stored: > iOutLevel (3 bits): An unsigned integer that specifies the outline level for this row. (which is consistent with 0-7 outline level) The equivalent XLS field is 2.4.221 `Row` and the [XLS spec](https://msdn.microsoft.com/en-us/library/dd906757.aspx) uses the same representation as XLS ### Other Formats ODS/FODS use nested `table:table-row-group` tags to indicate outline level SpreadsheetML and other formats have no support for grouping ### Tests In addition to manual verification, we'll also need to verify that round-tripping preserves the properties. Easiest way is to make a test file exploring corner cases and save in various formats. https://github.com/SheetJS/js-xlsx/blob/master/test.js#L1403 is the relevant part of the test suite for row properties (starting from a blank slate) https://github.com/SheetJS/js-xlsx/blob/master/test.js#L902 starts from the row_height test files. https://github.com/SheetJS/test_files/ search for row_height and you'll see: ``` row_height.biff5 row_height.slk row_height.xls row_height.xlsb row_height.xlsx row_height.xml ``` Based on quick tests, I suspect only the XLSX, XLSB, XLS, and BIFF5 (save as "Excel 5.0/95 Workbook") will be needed. ### Misc Don't worry about the code climate check -- we just removed it :) Can you unwind the changes to the dist files? those are updated when we cut releases. Note: We ended up having to fork the minifier to support extendscript. See https://github.com/mishoo/UglifyJS2/issues/1930 for the relevant issue. https://www.npmjs.com/package/@sheetjs/uglify-js is our fork.
Danwakeem commented 2017-07-10 04:12:30 +00:00 (Migrated from github.com)

XLSX

Based on the document you sent it looks like the default row outlineRow is 0 since at the bottom of that document in the attributes table it says

The possible values for this attribute are defined by the W3C XML Schema unsignedByte datatype.

and according to the W3C.org page, 3.4.24, the unsignedByte datatype starts at 0 minInclusive.

XLS / XLSB

I have made changes to support XLS / XLSB grouping based on the XLSB page you sent over and the XLS page you sent over. Those were very helpful thank you!

Although I did run into one problem, I don't see a way to generate XLS files in this library. Is that correct or am I missing something?

Other Formats

I will have to look into adding support for ODS/FODS as I am not familiar with these file types.

Tests

I will work on getting the test cases set up for this feature. Hopefully I can get this in tonight!

Misc

I unwinded the changes I made the the MakeFile. I will be sure to install that fork of uglify on my local machine in the future. Thanks for the heads up!

### XLSX Based on the [document you sent](https://github.com/SheetJS/js-xlsx/files/1134004/outlinelevel.pdf) it looks like the default row outlineRow is 0 since at the bottom of that document in the attributes table it says > The possible values for this attribute are defined by the W3C XML Schema unsignedByte datatype. and according to the [W3C.org](https://www.w3.org/TR/xmlschema11-2/#unsignedByte) page, 3.4.24, the unsignedByte datatype starts at 0 minInclusive. ### XLS / XLSB I have made changes to support XLS / XLSB grouping based on the XLSB [page you sent over](https://msdn.microsoft.com/en-us/library/dd773045.aspx) and the XLS [page you sent over](https://msdn.microsoft.com/en-us/library/dd906757.aspx). Those were very helpful thank you! Although I did run into one problem, I don't see a way to generate XLS files in this library. Is that correct or am I missing something? ### Other Formats I will have to look into adding support for ODS/FODS as I am not familiar with these file types. ### Tests I will work on getting the test cases set up for this feature. Hopefully I can get this in tonight! ### Misc I unwinded the changes I made the the `MakeFile`. I will be sure to install that fork of uglify on my local machine in the future. Thanks for the heads up!
SheetJSDev commented 2017-07-10 05:21:58 +00:00 (Migrated from github.com)

Wow! Those notes were mostly for me to go back later and review, definitely didn't expect you to go ahead and address the other formats too! 🎉 We can add some test files and roll the whole thing into one commit attributed to you :)

Check your inbox

Wow! Those notes were mostly for me to go back later and review, definitely didn't expect you to go ahead and address the other formats too! 🎉 We can add some test files and roll the whole thing into one commit attributed to you :) Check your inbox
SheetJSDev commented 2017-07-10 10:24:28 +00:00 (Migrated from github.com)

The commit was changed as follows:

  • the name is now level instead of groupLevel or outlineLevel -- the Excel UI labels the button "Group"/"Ungroup", some of the official documentation refer to it as "Outline Level", and the file specs exclusively refer to it as outline.

  • README was updated and a note was added about the discrepancy between the UI and the internal storage

  • The bit masks were fixed. For 3 bits, the mask is 7 (1 | 2 | 4). 15 is the mask for 4 bits (1|2|4|8)

  • ODS code has a new TODO, at some point we'll add ODS support but it's not worth stopping the next release.

  • Other miscellaneous changes to ensure that XLSX/XLSB row objects are written even if rows are empty.

  • The kitchen sink has an outline row -- if you run node tests/write.js you'll see the outline row in sheetjs.xlsx and sheetjs.xlsb

  • Added read tests and roundtrip tests. The read tests are based against the new outline files. It should also serve as a good test file for column outlines.

Even though the PR will show up as closed rather than merged, you should still be marked as a contributor.

The commit was changed as follows: - the name is now `level` instead of `groupLevel` or `outlineLevel` -- the Excel UI labels the button "Group"/"Ungroup", some of the official documentation refer to it as "Outline Level", and the file specs exclusively refer to it as outline. - [README was updated](https://sheetjs.gitbooks.io/docs/#row-properties) and a note was added about the discrepancy between the UI and the internal storage - The bit masks were fixed. For 3 bits, the mask is 7 (`1 | 2 | 4`). 15 is the mask for 4 bits (`1|2|4|8`) - ODS code has a new TODO, at some point we'll add ODS support but it's not worth stopping the next release. - Other miscellaneous changes to ensure that XLSX/XLSB row objects are written even if rows are empty. - The [kitchen sink](https://github.com/SheetJS/js-xlsx/blob/master/tests/write.js#L28) has an outline row -- if you run `node tests/write.js` you'll see the outline row in `sheetjs.xlsx` and `sheetjs.xlsb` - Added [read tests](https://github.com/SheetJS/js-xlsx/blob/master/test.js#L945) and [roundtrip tests](https://github.com/SheetJS/js-xlsx/blob/master/test.js#L1437). The read tests are based against the new [outline files](https://github.com/SheetJS/test_files/blob/master/outline.xlsx?raw=true). It should also serve as a good test file for column outlines. Even though the PR will show up as closed rather than merged, you should still be marked as a contributor.
Danwakeem commented 2017-07-10 13:29:26 +00:00 (Migrated from github.com)

Awesome, this is such a cool project and I am glad I could contribute! Thank you very much!

Awesome, this is such a cool project and I am glad I could contribute! Thank you very much!

Pull request closed

Sign in to join this conversation.
No description provided.