-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Fixed #1455 issue of border #1473
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
Conversation
Thank you for your submission. |
$cellStyle = $sheet->getStyle($column . $row); | ||
if (isset($attributeArray['rowspan'], $attributeArray['colspan'])) { | ||
$columnTo = $column; | ||
for ($i = 0; $i < (int) $attributeArray['colspan'] - 1; ++$i) { |
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.
Is the loop necessary? Or can the adjustment of the $columnTo value be done with simple mathematics, with conversion between character and numeric column addresses?
Might it be cleaner moving this calculation into a separate method, as it's duplicated for two cases?
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.
Yes Because it is incrementing the column which is A-Z and this is also used in the processDomElement on line number 504 so that is why i have used this. @MarkBaker. we can create separate method now as it is used total 4 times in the whole file.
$cellStyle = $sheet->getStyle($range); | ||
} elseif (isset($attributeArray['colspan'])) { | ||
$columnTo = $column; | ||
for ($i = 0; $i < (int) $attributeArray['colspan'] - 1; ++$i) { |
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.
Is the loop necessary? Or can the adjustment of the $columnTo value be done with simple mathematics, with conversion between character and numeric column addresses?
Which is the more efficient?
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.
Yes Because it is incrementing the column which is A-Z and this is also used in the processDomElement on line number 504 so that is why i have used this. @MarkBaker.
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.
Comments by Mark should be addressed and tests added before merging.
This is:
Checklist:
Why this change is needed?
This commit has been made to fix the #1455 issue of the border.
I have added range before applying the inline style instead of single cell to resolve border related issue #1455 .