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

Column Widths, Especially for ODS #3610

Merged
merged 2 commits into from
Jun 14, 2023
Merged

Conversation

oleibman
Copy link
Collaborator

@oleibman oleibman commented Jun 8, 2023

Fix #3609. Reporter created a spreadsheet with non-adjacent columns having non-default widths. Ods Writer needs to generate entries for the missing columns with default width.

The fix was fairly simple. Testing it was not. Ods Reader basically ignores all styles; they are complicated, declared in (at least) 2 places (content.xml and styles.xml). This change deals only with the problem as reported, in which the missing declarations should be in content.xml. If someone reports a real-world example of this involving styles.xml, I can look at that then. In the meantime, this toehold might serve as a template for adding other style processing to Ods Reader.

Looking at other formats, processing of column widths was also missing from Html Reader, and is now added. Note that this will work only with inline Css declarations on the col tags, which can be generated by Html Writer using setUseInlineCss(true). This creates a much larger file than one created without inline CSS.

A general problem became evident when studying the code. Worksheet columnDimensions is an unsorted array. This is not a problem per se, but it can easily lead to unexpected results from a getColumnDimensions call. The code is changed to sort the array before returning it in getColumnDimensions.

One existing test failed as a result of this change. It erroneously tested getHighestColumn instead of getHighestDataColumn, which caused a problem because the final column declaration included a number-columns-repeated attribute. The new result for getHighestColumn is correct, and the test is changed to use getHighestDataColumn instead, which was certainly its intent.

This is:

  • a bugfix
  • a new feature
  • refactoring
  • additional unit tests

Checklist:

  • Changes are covered by unit tests
    • Changes are covered by existing unit tests
    • New unit tests have been added
  • Code style is respected
  • Commit message explains why the change is made (see https://github.com/erlang/otp/wiki/Writing-good-commit-messages)
  • CHANGELOG.md contains a short summary of the change and a link to the pull request if applicable
  • Documentation is updated as necessary

Why this change is needed?

Provide an explanation of why this change is needed, with links to any Issues (if appropriate).
If this is a bugfix or a new feature, and there are no existing Issues, then please also create an issue that will make it easier to track progress with this PR.

oleibman added 2 commits June 7, 2023 21:40
Fix PHPOffice#3609. Reporter created a spreadsheet with non-adjacent columns having non-default widths. Ods Writer needs to generate entries for the missing columns with default width.

The fix was fairly simple. Testing it was not. Ods Reader basically ignores all styles; they are complicated, declared in (at least) 2 places (content.xml and styles.xml). This change deals only with the problem as reported, in which the missing declarations should be in content.xml. If someone reports a real-world example of this involving styles.xml, I can look at that then. In the meantime, this toehold might serve as a template for adding other style processing to Ods Reader.

Looking at other formats, processing of column widths was also missing from Html Reader, and is now added. Note that this will work only with inline Css declarations on the `col` tags, which can be generated by Html Writer using `setUseInlineCss(true)`. This creates a much larger file than one created without inline CSS.

A general problem became evident when studying the code. Worksheet `columnDimensions` is an unsorted array. This is not a problem per se, but it can easily lead to unexpected results from a `getColumnDimensions` call. The code is changed to sort the array before returning it in `getColumnDimensions`.

One existing test failed as a result of this change. It errorneously tested `getHighestColumn` instead of `getHighestDataColumn`, which caused a problem because the final column declaration included a `number-columns-repeated` attribute. The new result for `getHighestColumn` is correct, and the test is changed to use `getHighestDataColumn` instead, which was certainly its intent.
@oleibman oleibman merged commit 570660f into PHPOffice:master Jun 14, 2023
@oleibman oleibman deleted the issue3609 branch July 24, 2023 04:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Change column width not work
1 participant