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

Export column width sets on wrong column, when export to excel #511

Closed
Chibi86 opened this issue Jan 25, 2021 · 12 comments
Closed

Export column width sets on wrong column, when export to excel #511

Chibi86 opened this issue Jan 25, 2021 · 12 comments
Labels
bug slickgrid-universal external monorepo lib slickgrid-universal

Comments

@Chibi86
Copy link

Chibi86 commented Jan 25, 2021

I'm submitting a Bug report

Your Environment

Software Version(s)
Aurelia 1.31
Aurelia-Slickgrid 3.1.0
TypeScript 3.5.2
Operating System Windows 10
Node 10.15.3
NPM 6.4.1

Context

I try to setup export to excel, and also set export width for columns.

Expected Behavior

Title should get excel width on 50 point, when exporting.

Current Behavior

Screenshot_127
Title get default width on "8.43 points" in excel, when exporting.
Screenshot_126
But next column Duration gets 49.29 points, so it seems like width are of one column.

Possible Solution

What I can see when console.log out sheet from customExcelHeader, some extra header styles (mark with red in image) is set before the real ones. Which makes all columns styles be of one column I think.
Screenshot_129

Code Sample

this.columnDefinitions1 = [
 { id: 'title', name: 'Title', field: 'title', sortable: true, minWidth: 100, exportColumnWidth: 50 },
 { id: 'duration', name: 'Duration (days)', field: 'duration', sortable: true, minWidth: 100 },
 { id: '%', name: '% Complete', field: 'percentComplete', sortable: true, minWidth: 100 },
 { id: 'start', name: 'Start', field: 'start', formatter: Formatters.dateIso },
 { id: 'finish', name: 'Finish', field: 'finish', formatter: Formatters.dateIso },
 { id: 'effort-driven', name: 'Effort Driven', field: 'effortDriven', sortable: true, minWidth: 100 }
];
this.gridOptions1 = {
  enableAutoResize: false,
  enableSorting: true,
  enableExcelExport: true,
  registerExternalResources: [new ExcelExportService()]
};
@zewa666
Copy link
Collaborator

zewa666 commented Jan 25, 2021

Yes widths are defined per column. Also your 50px get converted to Excels pts with regards to default font. See https://docs.microsoft.com/en-us/office/troubleshoot/excel/determine-column-widths#:~:text=In%20a%20new%20Excel%20workbook,correctly%20set%20to%20Arial%2010.

@Chibi86
Copy link
Author

Chibi86 commented Jan 25, 2021

Yes widths are defined per column. Also your 50px get converted to Excels pts with regards to default font. See https://docs.microsoft.com/en-us/office/troubleshoot/excel/determine-column-widths#:~:text=In%20a%20new%20Excel%20workbook,correctly%20set%20to%20Arial%2010.

Did you really read the issue? I don't ask how export column width or excel width point works, I understand that. The issue is that export columns width is set on wrong column on export to excel.

@zewa666
Copy link
Collaborator

zewa666 commented Jan 25, 2021

Well your example set 50px to title. And your Excel Screenshot shows 8.43, which is the converted value. If you meant something different please elaborate

@Chibi86
Copy link
Author

Chibi86 commented Jan 25, 2021

Well your example set 50px to title. And your Excel Screenshot shows 8.43, which is the converted value. If you meant something different please elaborate

Export column width are not in pixels:
You can define a custom Excel column width (the width Excel's own width which is not in pixel). You can define a custom width per column (in your column definitions) and/or for the entire grid (in your grid options).
Export to excel in wiki

Here are the rest of the result:
Screenshot_132

One example where I remove export column width on title:
Screenshot_133

I been work with this issues for two work days, so I have try things like this already.

@ghiscoding
Copy link
Owner

@Chibi86 are you referring to this Excel Custom Column Width - Wiki? The default is 10, you can see it on this line and I don't know if it really uses pt or px, that would depends on how that external lib excel-builder.js, sizing the cell seems to be displayed in this page info on that lib. If you spot any errors then let me know, but if I look at my implementation, it will first use exportColumnWidth or else the column width but I don't think it will necessarily be precise though. I never use custom width, it was implemented by another user in my other lib.

Note excel-build.js is no longer supported by its original maintainer, the lib is still available and usable (and many other project like mine uses it) but don't expect any bug/fixes to ever be addressed on their side, even the docs are only available through the wayback machine since the original website no longer exists. So hopefully if there's anything, that would be in my code implementation

@Chibi86
Copy link
Author

Chibi86 commented Jan 25, 2021

@Chibi86 are you referring to this Excel Custom Column Width - Wiki? The default is 10, you can see it on this line and I don't know if it really uses pt or px, that would depends on how that external lib excel-builder.js, sizing the cell seems to be displayed in this page info on that lib. If you spot any errors then let me know, but if I look at my implementation, it will first use exportColumnWidth or else the column width but I don't think it will necessarily be precise though. I never use custom width, it was implemented by another user in my other lib.

Note excel-build.js is no longer supported by its original maintainer, the lib is still available and usable (and many other project like mine uses it) but don't expect any bug/fixes to ever be addressed on their side, even the docs are only available through the wayback machine since the original website no longer exists. So hopefully if there's anything, that would be in my code implementation

Yes, I referring to "Excel Custom Column Width - Wiki". I can confirm it takes pt, if I move column width in excel file to right column were I have calculate text width in pt it fits perfectly.

What I can see in excelExport.service.js:
If "grouping" is true it should add an extra columnStyle, which has the property "columnStyles" and the only one to have that in my example, is the extra one. I don't using column grouping, so it should not add an extra column style for that I think.

@ghiscoding
Copy link
Owner

I'm not exactly sure what you mean by grouping, but PR (Pull Request) are welcome, so if know how to fix then please submit a PR in Slickgrid-Universal, thanks.

@Chibi86
Copy link
Author

Chibi86 commented Jan 25, 2021

What I mean is the variable grouping which is declared on row 288 which is based on dataView.getGrouping().

@ghiscoding
Copy link
Owner

The thing is that you now talk about grouping but that is not anywhere in your very short code sample & print screen in your original question. So again I'm not sure why you talk about grouping now?

If you don't want to create a PR, you will have to at least provide a full code sample (on dumber or any other sandbox), I created that Excel Export Service 2 years ago, I don't fully recall on everything I wrote in that service. So please provide more steps and more code... or even better a PR to fix the issue.

@ghiscoding
Copy link
Owner

ghiscoding commented Jan 25, 2021

@Chibi86 ok I see the problem after testing that in any of the grid, what you were trying to say (I think) is that getGrouping() returns an array and just testing with if (grouping) is not sufficient, instead if should be if (Array.isArray(grouping) && grouping.length) { and after that it works as intended (as shown below).

I'll push a fix soon, I was about to push a new version this week anyway.

image

ghiscoding-SE pushed a commit to ghiscoding/slickgrid-universal that referenced this issue Jan 25, 2021
- the custom width was not applied to the correct column, when user had a custom width on column 1 it was applied to column 2. The bug was that getGrouping() always return an array and we should check for its length instead (not just if it exist)
- fixes issue report in Aurelia-Slickgrid ghiscoding/aurelia-slickgrid#511
@Chibi86
Copy link
Author

Chibi86 commented Jan 26, 2021

Thanks a lot for the help, even if I might have given to little information.

ghiscoding added a commit to ghiscoding/slickgrid-universal that referenced this issue Jan 26, 2021
…umn (#242)

- the custom width was not applied to the correct column, when user had a custom width on column 1 it was applied to column 2. The bug was that getGrouping() always return an array and we should check for its length instead (not just if it exist)
- fixes issue report in Aurelia-Slickgrid ghiscoding/aurelia-slickgrid#511
@ghiscoding ghiscoding added bug slickgrid-universal external monorepo lib slickgrid-universal labels Jan 28, 2021
@ghiscoding
Copy link
Owner

ghiscoding commented Jan 29, 2021

This is now fixed and released, however please note that you will have to update both Aurelia-Slickgrid v3.3.1 and all Slickgrid-Universal packages to 0.10.2 (there were some TS Typing changes that impacts both side).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug slickgrid-universal external monorepo lib slickgrid-universal
Projects
None yet
Development

No branches or pull requests

3 participants