NotificationsYou must be signed in to change notification settings
Fork1.9k
Star14.8k

Fix read this.worksheet before assign it#1934

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

Siemienik merged 2 commits intoexceljs:masterfromZyqGitHub1:master

Apr 14, 2023

Merged

Fix read this.worksheet before assign it#1934

Siemienik merged 2 commits intoexceljs:masterfromZyqGitHub1:master

Apr 14, 2023

Conversation

Copy link

Contributor

ZyqGitHub1 commentedDec 18, 2021

The anchor.js assignthis.worksheet = worksheet at end of constructor, but the getter (such as rowHeight) used in constructor will read this.worksheet. so it will cause rowHeight fall back to default value (180000);

Summary

When I want to add a image in a cell over a range, and I adjust the height of this row before. The nativeRowOff will not calculate use the row height.

constExcel=require('exceljs');constfs=require('fs').promises;(async()=>{constworkbook=newExcel.Workbook();awaitworkbook.xlsx.readFile('./test.xlsx');constworksheet=workbook.getWorksheet(1);constrow=worksheet.getRow(1);row.height=400;constcol=worksheet.getColumn('A');col.width=200constimage=awaitfs.readFile('./image.png')constimageId=workbook.addImage({buffer:image,extension:'png',});worksheet.addImage(imageId,{tl:{col:0.5,row:0.5},br:{col:1,row:1}});awaitworkbook.xlsx.writeFile('./result.xlsx');})()

result is

Test plan

After I change anchor.js,

constExcel=require('exceljs');constfs=require('fs').promises;(async()=>{constworkbook=newExcel.Workbook();awaitworkbook.xlsx.readFile('./test.xlsx');constworksheet=workbook.getWorksheet(1);constrow=worksheet.getRow(1);row.height=400;constcol=worksheet.getColumn('A');col.width=200constimage=awaitfs.readFile('./image.png')constimageId=workbook.addImage({buffer:image,extension:'png',});worksheet.addImage(imageId,{tl:{col:0.5,row:0.5},br:{col:1,row:1}});awaitworkbook.xlsx.writeFile('./result.xlsx');})()

the result is

The image top-left will start from 0.5*row height

Related to source code (for typings update)

https://github.com/ZyqGitHub1/exceljs/blob/458f372609589e6b3665a5c955998b677df2037e/lib/doc/anchor.js#L7

Fix: read this.worksheet before assign it

458f372

Siemienik self-assigned this

Apr 6, 2023

Siemienik approved these changes

Apr 14, 2023

View reviewed changes

Copy link

Member

Siemienik left a comment

Choose a reason for hiding this comment

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

Good catch! LGTM 🥇

Merge branch 'master' into master

4b6c0d0

Siemienik merged commit5f306b6 intoexceljs:master

Apr 14, 2023

Copy link

Contributor

hfhchan-plb commentedAug 9, 2023

This change means that all of the existing images are now off because the actual row height and col width are now being used. The previous behaviour was arguably a bug, but this is a breaking change causing every single image to be positioned differently.

Copy link

Contributor

hfhchan-plb commentedAug 9, 2023

There really should be an API which allows us to do actual absolute positioning, i.e. fixed pixel offset from the cell or fixed pixel offset from top left of spreadsheet, instead of the current absolute positioning which is more like relative.