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

Refactored autofill serial to remove duplication and exclude autofilling to readonly cells #31

Merged
merged 4 commits into from
Sep 19, 2017

Conversation

Xcone
Copy link

@Xcone Xcone commented Aug 2, 2017

Hi,

I encountered an issue when autofill would overlap readonly cells, and just wanted to quickly fix it. Then I learned of some duplication in this bit of code, and quite huge, hard to comprehend method. So I refactored that first. These commits are the result.

After testing I discovered a small functional change as well that had snuck in. When autofilling on merged cells it used to increment with the amount of merged cells, instead of treating them as one cell. I realised it actually makes more sense to me to count a merged cell as one cell, so I delibirately did not fix it.

Summary:

  • Cleaner code for AutoFill
  • AutoFill over merged cells counts better
  • AutoFill over readonly cells will not fill the content of the cell.

I hope you like it. :-)

Koen Visser added 2 commits July 10, 2017 23:56
…ells now only count as one diff increment instead of diff-times-rowspan increment.
@jingwood jingwood requested review from jingwood and removed request for jingwood August 2, 2017 23:15
…lso reduced redundancy of the message used in the RangeContainsReadonlyCellsException.
@Xcone
Copy link
Author

Xcone commented Aug 13, 2017

Updated the pull request. The AutoFill operation is now aborted by throwing exception, similar to the change in #32

@jingwood
Copy link
Member

jingwood commented Aug 14, 2017

@Xcone Thanks for the pull request and changes!

I just found a problem that exception happens when filling a serial that contains merged cell on every rows when using this pull request. I will check it and try to fix it. If you have any ideas about this please let me know.

@Xcone
Copy link
Author

Xcone commented Sep 4, 2017

Hi, I'm trying to reproduce it, but I have little info to go on. I tried some combinations of merged cells and apply fill serial, but I don't get any exception. Could you provide steps and/or a file with a range setup that can help me to reproduce it?

@jingwood
Copy link
Member

jingwood commented Sep 8, 2017

@Xcone, sorry for less details. Please try run this code:

var sheet = this.grid.CurrentWorksheet;

sheet.MergeRange("B3:D4");
sheet.MergeRange("B5:D6");
sheet["B3"] = 1;
sheet["B5"] = 2;

sheet.AutoFillSerial("B3:D6", "B7:D11");

Before perform the range filling, the sheet looks like this:
20170909 002544

…erforming a autofillserial-subset (a single row or column within the autofill command). Either can have no valid cells at this point whent the ranges contain merged cells.
@Xcone
Copy link
Author

Xcone commented Sep 16, 2017

Thanks. Found the exception now (it was hidden by UI before). And I fixed it as well.

P.S. Any indication when you'll repost the NuGet package containing these changes? Assuming it's all fixed now, of course ;-)

@jingwood
Copy link
Member

Thanks @Xcone, now it works greatly. I will public a NuGet package containing these changes.

@jingwood jingwood merged commit 4454105 into unvell:master Sep 19, 2017
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