Skip to content

Conversation

ndench
Copy link
Contributor

@ndench ndench commented Jun 28, 2018

Description

Element\Section seems to be the only element which doesn't allow you to set it's style with an object. This is a very small change that doesn't break BC, so that the style can be set with an object.

It makes it much easier to test as a consumer, because you can assert the style is the exact same object that you passed in. Instead of having to assert that you passed in the correct magic array by testing each attribute on the Style\Section object.

Checklist:

  • I have run composer run-script check --timeout=0 and no errors were reported
  • The new code is covered by unit tests (check build/coverage for coverage report)
  • I have update the documentation to describe the changes

Note:
I have not updated the documentation because it doesn't say anything about using style objects for any element. I am happy to contribute to the docs to add it though.

@coveralls
Copy link

coveralls commented Jun 28, 2018

Coverage Status

Coverage increased (+0.0009%) to 94.909% when pulling e05b6dc on ndench:section-style-object into 4fa9455 on PHPOffice:develop.

$this->setDocPart($this->container, $this->sectionId);
$this->style = new SectionStyle();
$this->setStyle($style);
if (null === $style) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't you remove this check, and pass true in setNewStyle instead?

@troosan troosan added this to the v0.16.0 milestone Dec 8, 2018
@troosan troosan merged commit cf3132a into PHPOffice:develop Dec 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants