Skip to content

Xlsx style cell column #16066

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

Open
wants to merge 3 commits into
base: 4.x
Choose a base branch
from
Open

Conversation

gpibarra
Copy link
Contributor

Description

Currently, you can style the header and the rest of the cells (separately). This PR allows you to format each of the exported columns (value cells only; header row cells are not included).
When you style a column, you can call the function multiple times, and each style (or callback) is saved in an array. When obtaining the column style, all styles are calculated and merged. This is designed to allow styling, for example, in the make/setup function for potential new column classes that extend the ExportColumn class.
Styles (or callbacks) can be fixed, meaning they do not depend on the cell value. They will be cached to improve performance and avoid merging each row of data in the export file. Additionally, you can assign a style based on the cell value, which will be executed on each row of data (OpenSpout does not support Excel conditional formatting).

Functional changes

  • Code style has been fixed by running the composer cs command.
  • Changes have been tested to not break existing functionality.
  • Documentation is up-to-date.

@gpibarra
Copy link
Contributor Author

In the documentation, I corrected a change to this PR: #16025
It wasn't clear whether the function needed to be renamed or not, but only the documentation had been changed. Feel free to rename the function.

@danharrin danharrin added this to the v4.0.0-beta1 milestone Apr 16, 2025
Copy link
Member

@danharrin danharrin left a comment

Choose a reason for hiding this comment

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

To be honest I am concerned about the performance implications of calling another method for each cell. Maybe the implementation should just be makeXlsxRow() static method on the Exporter where you can pass a function that accepts $data, $style, $columns and returns a Row object. The $columns would be an array of column names that match the $data in order, so you can use Row::fromValuesWithStyles() yourself. If that function is not passed, the makeXlsxRow() would just return the default Row::fromValues() object. This way, we aren't really introducing new overhead by looping over columns etc. Thoughts?

@github-project-automation github-project-automation bot moved this from Todo to In Progress in Roadmap Apr 17, 2025
@danharrin
Copy link
Member

At this point, maybe the entire writer creation process should just be moved onto the Exporter class, so it can be overriden in any way instead of needing all these new config methods

@gpibarra
Copy link
Contributor Author

I agree. Initially, I only called getStyle once before the record loop. But the fact that the library doesn't support conditional formatting motivated me to create an additional method. I think it's perhaps too much customization for the default case. I would leave the basic style and not the one that depends on the value (called for each cell in each row).

Regarding the writer depending on the exporter, I don't see it as a problem, but if not, it's not that difficult to override the logic. These changes I proposed were born from a project with Filament v3, and I modified the writer logic with a bind from a service provider. It's not complicated.

@danharrin
Copy link
Member

Can yon move all the writer logic into the exporter class then please? We could even remove that configureXlsxWriterBeforeClose() method tbh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request low priority
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

2 participants