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

Implement per cell styling for openspout #359

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

jschram
Copy link

@jschram jschram commented Apr 25, 2024

It would be nice to be able to format certain values as numeric, date, etc.
To achieve this, DateTime values should be accepted as well, from within Exportable::transformRows. There already is support in openspout for DateTime values (see: Writer\XLSX\Manager\WorksheetManager). So, I've made a contribution to openspout/openspout that allows a full Row to be created from an array with styling per cell. This was merged today and a new version has been tagged. See: openspout/openspout#235

This PR implements this change, allowing users to set the formatting of certain columns (based on their numeric index) when exporting data.

For instance:

(new FastExcel(clone $collection))
    ->setColumnStyles([
        0 => (new Style())->setFormat('mm/dd/yyyy'),
    ])
    ->export($file);

@jschram
Copy link
Author

jschram commented May 10, 2024

Hi @rap2hpoutre! Is there anything I can do to aid you in reviewing this PR? Please let me know if you need further explanation!

@jschram
Copy link
Author

jschram commented May 22, 2024

Hi @rap2hpoutre, gentle reminder and honest question: Is this package still being maintained?

@potsky
Copy link

potsky commented May 27, 2024

Hi @jschram

Can't wait to test this!
Do you know if we can handle sheets?
We generate files with several sheets and need different custom styles on each sheet.

@jschram
Copy link
Author

jschram commented May 28, 2024

Hi @potsky

Can't wait to test this! Do you know if we can handle sheets? We generate files with several sheets and need different custom styles on each sheet.

Not quite sure, but I can't see why it wouldn't support multiple sheets. I've changed the way the rows are generated so you can provide an array with styles per column. As long as these column styles match for all sheets, I think you'll be fine. When those have to differ, I think it might need some more work.

Could you test it out and let me know if it does indeed work?

@potsky
Copy link

potsky commented May 28, 2024

Hi @potsky

Can't wait to test this! Do you know if we can handle sheets? We generate files with several sheets and need different custom styles on each sheet.

Not quite sure, but I can't see why it wouldn't support multiple sheets. I've changed the way the rows are generated so you can provide an array with styles per column. As long as these column styles match for all sheets, I think you'll be fine. When those have to differ, I think it might need some more work.

Could you test it out and let me know if it does indeed work?

Yep I will test your PR.
The idea is to have a style different on the first sheet and an other on the second.
We could add $sheetIndex as an optional second argument like this if I get your example:

(new FastExcel(clone $collection))
    ->setColumnStyles([
        0 => (new Style())->setFormat('mm/dd/yyyy'),
    ], $sheetIndex)
    ->export($file);

I can make a PR on your PR if you want me to add this, just let me know!

@jschram
Copy link
Author

jschram commented May 28, 2024

Yep I will test your PR.

Awesome, thanks!

The idea is to have a style different on the first sheet and an other on the second. We could add $sheetIndex as an optional second argument like this if I get your example:

(new FastExcel(clone $collection))
    ->setColumnStyles([
        0 => (new Style())->setFormat('mm/dd/yyyy'),
    ], $sheetIndex)
    ->export($file);

I can make a PR on your PR if you want me to add this, just let me know!

Sounds good. Yeah, the $column_styles property in Exportable could be multi-dimensional. As long as you're careful to pass the right set of column styles when writing a row, it would be a nice and generic solution 👍

@potsky
Copy link

potsky commented May 28, 2024

@jschram it works!
Finally we have found an other way for our multi-sheet so we will not add more feature.

@jschram
Copy link
Author

jschram commented May 28, 2024

@jschram it works!

Glad to hear, thanks a lot for testing!

@jschram
Copy link
Author

jschram commented May 28, 2024

So with all checks having passed, and a successful manual test, maybe this PR can be merged now, @rap2hpoutre? 🙏

Would be really great if I can use this change from your repository for my project instead of relying on my fork.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants