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

Conditional styles comming separates, even though two of them are one conditional style when you open the sheet #4039

Closed
bokrma opened this issue May 22, 2024 · 2 comments · Fixed by #4042

Comments

@bokrma
Copy link

bokrma commented May 22, 2024

when I tried to import a sheet using the function:
$this->worksheet->getConditionalStylesCollection();
the data inside of it was splitter by ranges
but in reality there are two ranges belong to one set of conditional styling
can you please provide a general function that fetches the conditional stylings as its defined?
Conditional styles test.xlsx
this is a test file to check with

@oleibman
Copy link
Collaborator

Thank you for providing a sample file. In it, the style is stored as:

<conditionalFormatting sqref="B1:B10 C1:C10">

I am not sure why it is not stored as:

<conditionalFormatting sqref="B1:C10">

If the latter form were used, you would only see a single style, as you wish. Since the former form is used (and I don't know why that is the case), we do our best to interpret the split range, and "our best", in this case, is to treat it as 2 separate ranges. As far as I can tell, this still results in a correct spreadsheet, with the conditional working correctly for both ranges.

I confess that I am not sure why we need to split the range, but I don't know if not splitting it will lead to regression problems. Before I begin investigating, can you tell me why it matters to you?

@bokrma
Copy link
Author

bokrma commented May 23, 2024

the idea of having it as it's defined, is that if the user import it to our system, we need to know if its defined as signluar or 2 ranges, or multiple (since it could be many 4-5 ranges, etc)
and our code generates the database related to it and shows it in a frontend like Excel sheet
Screenshot 2024-05-23 at 11 20 36

Screenshot 2024-05-23 at 11 21 37

oleibman added a commit to oleibman/PhpSpreadsheet that referenced this issue May 24, 2024
Fix PHPOffice#4039. Excel sometimes stores the location for a Conditional as, say, `B1:B10 C1:C10` rather than `B1:C10`. PhpSpreadsheet does not have a problem with this, but internally stores it as separate Conditionals, one for each range. (There may be more than 2.) User would like it stored as a single Conditional. Since the range is used as the index of an array which holds the conditionals, there is no technical reason why this can't be done. And it does seem like being able to change multiple ranges all at once has some advantages, whether you're doing it in PhpSpreadsheet or in Excel itself.

Making such a change in Xlsx Reader showed no problem in Xlsx Reader and Writer tests. A number of problems did show up in CellMatcherTest, and one in WizardFactoryTest. This was good news, because it meant some of our test spreadsheets used the same type of construction as the test spreadsheet supplied with the issue. So, if additional fixes make the test problems go away, I think no new tests are required. And a smattering of source changes did indeed result in a clean test suite once more.

This is a change, but I don't really think it should break anyone. So I don't think it's necessary to add an option  to opt in to the old or new behavior. I could be wrong. I'll leave this ticket open for at least a couple of weeks to see if anyone thinks otherwise.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants