-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Active Cell Anywhere #1323
Active Cell Anywhere #1323
Conversation
When freeze pane is in use on a worksheet, PhpSpreadsheet saves to Xlsx in such a way that the active cell is always set to the top left cell below the freeze pane. I find it difficult to understand why: 1. You have given users the setSelectedCells function, but then choose to ignore it. 2. Excel itself does not act in this manner. 3. PHPExcel did not act in this manner. 4. PhpSpreadsheet when writing to Xls does not act in this manner. This is especially emphasized because the one test in FreezePaneTest which would expose the difference is the only test in that member which is not made for both Xls and Xlsx. 5. It is *really* useful to be able to open a spreadsheet anywhere, even when it has header rows. Rather than create regression problems by changing this behavior all the time, I instead introduce a new method, setActiveCellAnywhere, to Writer/Xlsx/Worksheet to control it via a private variable, with the default (false) being the current behavior. When it is set to true, the active cell is left as the user intended. The documentation is updated to describe the new method. New tests are added, and the Xlsx-only test is changed to test both Xls and Xlsx.
Apply Scrutinizer recommendations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the PR. As you mentioned yourself it would be best to have xls and xlsx writers kept in sync as much as possible. As such is best to modify existing (incorrect) behavior, rather than add a new method.
Could you please edit your PR in that way ? That would probably make it much simpler and smaller.
Not knowing best way to handle potential changes to existing behavior, existing change is more complex than needed by adding a new method to override existing behavior only on request. PowerKiKi reviewed request, and asked that it be simplified by modifying existing behavior, eliminating the need for override method. This change modifies the PR as requested.
Change simplified per request. |
Travis CI reported problem with Calculation. That was changed in master 3 days ago (delete some unused code). Perhaps the lack of that change is the problem here.
I noticed that Calculation had been changed 3 days ago. Manually applying that simple change to my pull request eliminated the Travis CI problem. I don't know if that was the best way to solve the problem or not, but it worked. |
Thanks, merged in fb37938 |
When freeze pane is in use on a worksheet, PhpSpreadsheet saves to Xlsx in such a way that the active cell is always set to the top left cell below the freeze pane. I find it difficult to understand why: 1. You have given users the setSelectedCells function, but then choose to ignore it. 2. Excel itself does not act in this manner. 3. PHPExcel did not act in this manner. 4. PhpSpreadsheet when writing to Xls does not act in this manner. This is especially emphasized because the one test in FreezePaneTest which would expose the difference is the only test in that member which is not made for both Xls and Xlsx. 5. It is *really* useful to be able to open a spreadsheet anywhere, even when it has header rows. Closes PHPOffice#1323
When freeze pane is in use on a worksheet, PhpSpreadsheet saves to Xlsx in such
a way that the active cell is always set to the top left cell below the freeze
pane. I find it difficult to understand why:
ignore it.
This is especially emphasized because the one test in FreezePaneTest which
would expose the difference is the only test in that member which is
not made for both Xls and Xlsx.
it has header rows.
Rather than create regression problems by changing this behavior all the time,
I instead introduce a new method, setActiveCellAnywhere, to
Writer/Xlsx/Worksheet to control it via a private variable, with the default
(false) being the current behavior. When it is set to true, the active cell
is left as the user intended.
The documentation is updated to describe the new method.
New tests are added, and the Xlsx-only test is changed to test
both Xls and Xlsx.
This is:
Checklist:
Why this change is needed?
As described above, add the ability to open a worksheet anywhere even when freeze pane is used.