-
Notifications
You must be signed in to change notification settings - Fork 591
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
expand columns to get correct displayed value instead of compressed cell value
- Loading branch information
Showing
5 changed files
with
15 additions
and
3 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ba65bf7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ruthtoh I haven't tried the code but I can imagine that it expands the columns maybe because unexpanded columns return missing data. Want to add a note to see if u can consider testing some of the tests below to make sure the change doesn't break existing use cases.
#1081 (comment)
ba65bf7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw nice implementation you made! @ruthtoh
ba65bf7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also copying @ruthtxh because I'm not sure if your new account gets notifications
ba65bf7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @kensoh
This fix is for the issue flagged up by @lookang as some of his date values get extracted as ####### instead of the date itself.
The root cause is we are taking the displayed value of the cell due to previous issues as discussed here.
So to overcome this while still taking the displayed value, one way is to programmatically expand the cells to autofit.
I've done some tests before pushing the codes but I can test further to see if there's any edge case.
P.S. I pushed the codes from my mac and there's some problem with my github desktop there that keeps connecting to this stray account @ruthtoh. But I'm primarily using my current one!
ba65bf7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ruthtxh oh I see... Do you mean that columns which are too narrow, on Mac Excel it will display as ####### for dates, and thus the extracted data to TagUI will be ####### instead of proper date?
If that's the case, I would think that the adjustment should be done from user's side, by setting the column width correctly so that the displayed value is shown properly, and extracted properly. Otherwise this change will mean that it will modify the user Excel spreadsheet every time, to autofit all columns, since a save is done at the end.
I would think modifying user's column width formatting is a bad idea as that is not something most users want and ask for, and I'll imagine that this change to solve an edge case might give a negative surprise to more users. In general, modifying user's source data is not a good idea. This is my recommendation, but will leave it to you, and perhaps Lawrence to decide on the best approach.
ba65bf7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried changing on my Mac Windows Excel, the width of the column, but it continues to be reopened in the flow the smaller width despite having save it in Excel in The wider width.
ba65bf7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I speculate the TagUI 6.110 doesn't modify my data, just make it correctly read by auto adjusting the width of column. I think it is a good change.
I am okay with @ruth changes so far.
ba65bf7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Lawrence! Gotcha, looks like in absence of other Mac-Excel users feedback, your feedback will be the most accurate and if data or formatting is not corrupted then maybe the right approach is the leave the change there and see how it iterates from there.
ba65bf7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kensoh thanks again for your awesome tool and your kind understanding.
I think it should a good change to avoid other users getting ###### when the data is actually in numbers like date and time.
ba65bf7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kensoh @lookang
Ok we will keep a lookout for future feedbacks and iterate from here.