-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Console] added TableCellStyle #37338
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
b2bb8e3
to
1bf214a
Compare
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.
LGTM (minor comments)
$options = $this->getOptions(); | ||
|
||
return isset($options['align']) | ||
&& self::DEFAULT_ALIGN !== $options['align']; |
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.
should on the same line
|
||
public function hasAlign(): bool | ||
{ | ||
$options = $this->getOptions(); |
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.
can be inlined
return $this->options['style']; | ||
} | ||
|
||
public function hasAlign(): bool |
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 would remove this method
&& $style->hasAlign(); | ||
} | ||
|
||
public function hasCellFormat(): bool |
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 would remove this method as well
{ | ||
$options = $this->getOptions(); | ||
|
||
return isset($options['cellFormat']) && \is_string($options['cellFormat']); |
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.
Same as above
700c4ba
to
dd70d46
Compare
@fabpot made edits |
$rows[$line][$column] = new TableCell($lines[0], ['colspan' => $cell->getColspan()]); | ||
$rows[$line][$column] = new TableCell( | ||
$lines[0], ['colspan' => $cell->getColspan(), 'style' => $cell->getStyle()] | ||
); |
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.
Can you move it on one line again?
$unmergedRows[$unmergedRowKey][$column] = new TableCell($value, ['colspan' => $cell->getColspan()]); | ||
$unmergedRows[$unmergedRowKey][$column] = new TableCell( | ||
$value, ['colspan' => $cell->getColspan(), 'style' => $cell->getStyle()] | ||
); |
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.
Can you move it on one line again?
@@ -33,6 +34,10 @@ public function __construct(string $value = '', array $options = []) | |||
throw new InvalidArgumentException(sprintf('The TableCell does not support the following options: \'%s\'.', implode('\', \'', $diff))); | |||
} | |||
|
|||
if (isset($options['style']) && !$options['style'] instanceof TableCellStyle) { | |||
throw new InvalidArgumentException('The style option must be instance of TableCellStyle.'); |
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.
must be an instance of
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.
"TableCellStyle"
} | ||
|
||
/** | ||
* @return array|string[] |
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.
@return string[]
should be enough, no?
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.
Would that work?
@fabpot done |
return isset($this->getOptions()['align']) && self::DEFAULT_ALIGN !== $this->getOptions()['align']; | ||
} | ||
|
||
public function hasCellFormat(): bool |
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.
Same for this one.
return $this->getOptions()['cellFormat']; | ||
} | ||
|
||
public function hasAlign(): bool |
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'm still not convinced that we should keep this method
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 think the both methods can be deleted. Updated
} | ||
|
||
/** | ||
* @return array|string[] |
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.
Would that work?
0f1aaa5
to
6b13b9f
Compare
6b13b9f
to
aff7628
Compare
Thank you @khoptynskyi. |
This PR was merged into the master branch. Discussion ---------- [Console] added TableCellStyle documentation See symfony/symfony#37338 Commits ------- 4a5be12 [Console] added TableCellStyle documentation
Added an opportunity to customize a table cell via TableCellStyle object.
It can be used as
See #13370