Skip to content

fix: preserve template style when using a converter #402

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

Conversation

HaceraI
Copy link

@HaceraI HaceraI commented Jul 4, 2025

This pull request addresses an issue where pre-existing cell styles from a template are lost when the cell's data is processed by a Converter. I have created a minimal reproducible example to demonstrate the problem.

The Problem

When writing data to an Excel file using a template that contains pre-formatted cells (e.g., with background colors, borders), if a cell's content is handled by a Converter (like LocalDateConverter which applies a date format), the original style from the template is completely overridden. The final cell only retains the new style elements introduced by the converter (typically just the data format), losing all visual styling from the template.

To Reproduce:

I have set up a dedicated repository that clearly demonstrates this issue. It uses a template with a yellow background in cell B2 and writes a LocalDate to it.

Root Cause Analysis

The root cause lies in the interaction between Converter implementations and the FillStyleCellWriteHandler.

  1. Most Converter implementations (e.g., for Date, LocalDate) call WorkBookUtil.fillDataFormat() to apply a specific data format.
  2. This utility creates a new WriteCellStyle object and sets it on the WriteCellData.
  3. Crucially, the originCellStyle on the WriteCellData remains null, as the framework does not automatically capture the cell's existing style from the template at this stage.
  4. Later, in FillStyleCellWriteHandler, the handler attempts to merge writeCellStyle (from the converter) and originCellStyle (null). Since originCellStyle is null, the template's style is ignored, and the cell's style is completely replaced by the new style from the converter.

The Solution

To address this, I've made a targeted modification to FillStyleCellWriteHandler.

The new logic introduces a check:

if (originCellStyle == null && writeCellStyle != null && writeCellStyle.getDataFormatData() != null) {
    originCellStyle = context.getCell().getCellStyle();
}

This is a surgical fix that ensures:

  • It only intervenes when originCellStyle has not been set.
  • It specifically targets scenarios where a Converter has likely added a data format (writeCellStyle.getDataFormatData() != null).
  • It safely captures the existing style directly from the POI Cell object before the merge operation.

This approach is minimally invasive and avoids breaking changes to the Converter interface or other utility classes.

@HaceraI HaceraI marked this pull request as ready for review July 7, 2025 06:40
@HaceraI HaceraI changed the title fix(style): Preserve template style when using a converter fix: preserve template style when using a converter Jul 7, 2025
@delei
Copy link
Collaborator

delei commented Jul 8, 2025

Could you please provide a unit test case?
能否提供一个对应的单元测试案例?

@HaceraI
Copy link
Author

HaceraI commented Jul 8, 2025

I noticed that the existing test cases seem to address or explain this issue, so perhaps my change is unnecessary?
在测试用例中,我发现似乎存在对该问题的说明及解决方案,也许我的更改是不必要的?

Link

@delei
Copy link
Collaborator

delei commented Jul 8, 2025

Hi, @HaceraI ,thanks for submitting this PR.As for now, this PR has not started review yet.
IMO, if necessary,you can try to improve the unit tests first.If it's just a matter of usage, or this code comment provides a solution.You can decide for yourself whether to close this PR.

感谢提交这个 PR,目前这个还没开始 Review
IMO,如果必要的话,可以先尝试改进单元测试。如果只是使用上的问题,或者代码注释已经提供了解决方案,你可以自行决定是否关闭这个PR。

@HaceraI HaceraI closed this Jul 11, 2025
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.

2 participants