Skip to content

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Fixed #1455 issue of border #1473

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
782d07e
Fixed #1455 issue of border
dhaval247 May 13, 2020
1f66275
Added Test case for fixing #1455 issue
dhaval247 Jul 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion src/PhpSpreadsheet/Reader/Html.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -699,7 +699,26 @@ private function applyInlineStyle(&$sheet, $row, $column, $attributeArray)
return;
}

$cellStyle = $sheet->getStyle($column . $row);
if (isset($attributeArray['rowspan'], $attributeArray['colspan'])) {
$columnTo = $column;
for ($i = 0; $i < (int) $attributeArray['colspan'] - 1; ++$i) {
Copy link
Member

@MarkBaker MarkBaker May 23, 2020 •
edited
Loading

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?

Copy link
Contributor Author

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.

++$columnTo;
}
$range = $column . $row . ':' . $columnTo . ($row + (int) $attributeArray['rowspan'] - 1);
$cellStyle = $sheet->getStyle($range);
} elseif (isset($attributeArray['rowspan'])) {
$range = $column . $row . ':' . $column . ($row + (int) $attributeArray['rowspan'] - 1);
$cellStyle = $sheet->getStyle($range);
} elseif (isset($attributeArray['colspan'])) {
$columnTo = $column;
for ($i = 0; $i < (int) $attributeArray['colspan'] - 1; ++$i) {
Copy link
Member

@MarkBaker MarkBaker May 23, 2020 •
edited
Loading

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?

Copy link
Contributor Author

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.

++$columnTo;
}
$range = $column . $row . ':' . $columnTo . $row;
$cellStyle = $sheet->getStyle($range);
} else {
$cellStyle = $sheet->getStyle($column . $row);
}

// add color styles (background & text) from dom element,currently support : td & th, using ONLY inline css style with RGB color
$styles = explode(';', $attributeArray['style']);
Expand Down
31 changes: 31 additions & 0 deletions tests/PhpSpreadsheetTests/Reader/HtmlTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,4 +424,35 @@ public function testTextIndentUseRowspan()
self::assertEquals(10, $style->getAlignment()->getIndent());
unlink($filename);
}

public function testBorderWithRowspanAndColspan()
{
$html = '<table>
<tr>
<td style="border: 1px solid black;">NOT SPANNED</td>
<td rowspan="2" colspan="2" style="border: 1px solid black;">SPANNED</td>
</tr>
<tr>
<td style="border: 1px solid black;">NOT SPANNED</td>
</tr>
</table>';

$reader = new Html();
$spreadsheet = $reader->loadFromString($html);
$firstSheet = $spreadsheet->getSheet(0);
$style = $firstSheet->getStyle('B1:C2');

$borders = $style->getBorders();

$totalBorders = [
$borders->getTop(),
$borders->getLeft(),
$borders->getBottom(),
$borders->getRight(),
];

foreach ($totalBorders as $border) {
self::assertEquals(Border::BORDER_THIN, $border->getBorderStyle());
}
}
}