-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Add support for style objects to the element constructor #2362
base: master
Are you sure you want to change the base?
Conversation
The current version does not support passing style objects to the constructor of an element. Only arrays are supported. But for a full OOP approach the element constructor should also support style objects. As the setNewStyle method is only used internally, the returnObject param can be ignored when passing a style element instance as styleValue via constructor.
I'll cover this one with unit tests until the end of this week. |
…a variable as it is only used in one specific method.
…sed in the element's constructor.
…ularExpression assertion over the deprecated assertRegExp.
UnitTests added, documentation modified. You're welcome! |
…cell to actually cover the new style object support via constructor.
While I was working on this issue I realized that the style behavior should be revised. The constructor and set style methods should only support the actual style object. The array support should be abandoned. Instead a static fromArray method should be added to each style. Before: $cell = new cell(100, [
// some style
]); After: $cell = new cell(100, CellStyle::fromArray([
// some style
]); Since this would be a breaking change, I decided to go for the workaround committed here to at least add support for style objects for elements like cell. I've also noticed that the style behavior is quite different across the various element types. The text object for example always uses the same object passed via constructor. Other elements prefer cloning. I think that we should also add a merge method to element styles and a dedicated className array. All elements should always return a style object via getStyle() method. [...]
public function addClassName(string $className): void;
public function hasClassName(string $className): bool;
public function clearClassNames(): void;
public function hasClassNames(): bool;
public function getClassNames(): array;
public function getStyle(): <StyleObject>;
[...] With the merge method something like this could happen in the renderer: $style = $element->getStyle();
foreach ($phpWord->getStyles($element->getStyleNames() as $classStyle) {
if ($classStyle::class === $style::class) {
$style->merge($classStyle);
}
} This would allow adding multiple class names to an object but also overwrite specific attributes it. My private html2word library does support multiple classes in a similar way, but I don't know how Word files are actually rendered. If Word only supports one class name per element, then passing multiple classes to an element doesn't make sense. But even then, the set style method should only support `string|'. |
2d9f999
to
e458249
Compare
Description
The current version does not support passing style objects to the constructor of an element. Only arrays are supported. But for a full OOP approach the element constructor should also or only support style objects. As the setNewStyle method is only used internally, the returnObject param can be ignored when passing a style element instance as styleValue via constructor.
Fixes # (issue)
Elements do not support style objects via constructor.
Checklist:
composer run-script check --timeout=0
and no errors were reported