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

#1405, add new function SetSheetBackgroundFromBytes #1406

Merged
merged 4 commits into from
Dec 1, 2022

Conversation

jianxinhou
Copy link
Contributor

@jianxinhou jianxinhou commented Nov 28, 2022

PR Details

Description

Because of business need, We added a new method for struct File:SetSheetBackgroundFromBytes.

the function of this method is similar with existing method(SetSheetBackground), they both can add background image for excel files, but the difference between SetSheetBackground and SetSheetBackgroundFromBytes is, the new method use binary image data as image source. And SetSheetBackgroundFromBytes do not need to specify the extension of image data, It can determine the image format by itself.

由于业务需要, 我们为File结构体增加了一个新的方法:SetSheetBackgroundFromBytes

该方法的功能与现存SetSheetBackground方法相同,即添加背景图像,不同的是新增方法支持使用二进制数据设置背景图像,同时,SetSheetBackgroundFromBytes无需指定文件后缀,该方法可自行判断二进制数据中所包含图像的格式。

Related Issue

#1405

Motivation and Context

In our business, we want to set the binary image data requested from the upstream service as the background of the excel file directly, so we implement SetSheetBackgroundFromBytes.

在业务场景中,我们希望能够将从上游服务中请求到的二进制图片数据直接设置为excel文件的背景,因此实现了SetSheetBackgroundFromBytes

How Has This Been Tested

  • We loaded all the images under test/images/ and excelize.svg, and called SetSheetBackgroundFromBytes to set the binary data loaded into memory as the background image, the result shows that this method can distinguish the binary data format and set for background, see TestSetBackgroundFromBytes in sheet_test.go;

  • In addition to the above images, we also tested with other positive and negative example files that are not in this project, and the results show that SetSheetBackgroundFromBytes can be executed correctly;

  • We found that when we use the image test/images/excel.wmz as the background, no matter calling the existing SetSheetBackground or the added SetSheetBackgroundFromBytes, the set background cannot be displayed correctly, maybe there is a problem with the image itself;

  • We found that when we use the .svg file as the background image, it cannot be displayed normally in excel, and you cannot manually set the .svg file as the background image in excel. It seems that excel does not support setting the .svg file as the background image, but it can be displayed normally in WPS.

  • 我们读取了test/images/下的所有图像以及excelize.svg,并调用SetSheetBackgroundFromBytes将加载到内存的二进制数据设置为背景图像,结果显示该方法能正常区分二进制数据格式并设置背景,详见sheet_test.go中的TestSetBackgroundFromBytes

  • 除上述图像外,我们还使用其他不在本项目中的正例和反例文件做了测试,结果显示SetSheetBackgroundFromBytes均能够正确执行;

  • 我们发现使用图像test/images/excel.wmz作为背景时,无论调用现存的SetSheetBackground还是新增的SetSheetBackgroundFromBytes都无法正确显示设置的背景,可能是图像本身有问题;

  • 我们发现使用.svg文件作为背景图像时,在excel中无法正常显示,同时在excel中也无法手动设置.svg文件作为背景图像,似乎excel并不支持设置.svg文件作为背景,但在WPS中可以正常显示。

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

… used for setting background image by given image data

Change-Id: I83baac1107f66dfa5d9d2eebfa7e40f9f49b79d5
@jianxinhou jianxinhou changed the title This closes #1405, add new function SetSheetBackgroundFromBytes used … This closes #1405, add new function SetSheetBackgroundFromBytes Nov 28, 2022
@jianxinhou jianxinhou changed the title This closes #1405, add new function SetSheetBackgroundFromBytes #1405, add new function SetSheetBackgroundFromBytes Nov 28, 2022
@xuri xuri added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 28, 2022
Copy link
Member

@xuri xuri left a comment

Choose a reason for hiding this comment

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

Thanks for your pull request. I've left some comments.

picture.go Outdated Show resolved Hide resolved
sheet_test.go Outdated Show resolved Hide resolved
sheet.go Outdated Show resolved Hide resolved
houjianxin.rupert added 2 commits November 29, 2022 11:08
Change-Id: I04479f92c559e4bbfa5e81d5698389122c90743c
Change-Id: If259d00c12f2b30ebd8dba20845ddf92dc9bc717
sheet.go Outdated Show resolved Hide resolved
sheet_test.go Outdated Show resolved Hide resolved
sheet_test.go Outdated Show resolved Hide resolved
sheet.go Show resolved Hide resolved
sheet.go Outdated Show resolved Hide resolved
sheet.go Outdated Show resolved Hide resolved
@xuri xuri added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 30, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #1406 (85d8a88) into master (dde6b9c) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1406      +/-   ##
==========================================
- Coverage   98.54%   98.53%   -0.02%     
==========================================
  Files          31       31              
  Lines       23936    23947      +11     
==========================================
+ Hits        23588    23596       +8     
- Misses        230      232       +2     
- Partials      118      119       +1     
Flag Coverage Δ
unittests 98.53% <100.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
picture.go 98.72% <ø> (+0.01%) ⬆️
sheet.go 100.00% <100.00%> (ø)
chart.go 97.93% <0.00%> (-2.07%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@xuri xuri left a comment

Choose a reason for hiding this comment

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

I have fixed some code review issues and updates the unit test case and documentation for this based on your branch. Please upgrade to the master branch code, and this feature will be released in the next version.

@xuri xuri merged commit 5e0953d into qax-os:master Dec 1, 2022
xuri pushed a commit to JDavidVR/excelize that referenced this pull request Jul 11, 2023
jenbonzhang pushed a commit to jenbonzhang/excelize that referenced this pull request Oct 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add new function SetSheetBackgroundFromBytes used for setting background image by given image data
3 participants