Support for configuring a Chart Title's overlay

[u01jmg3]

This is:

• a bugfix
• a new feature
• refactoring
• additional unit tests

Checklist:

• Changes are covered by unit tests
  • Changes are covered by existing unit tests
  • New unit tests have been added
• Code style is respected
• Commit message explains why the change is made (see https://github.com/erlang/otp/wiki/Writing-good-commit-messages)
• CHANGELOG.md contains a short summary of the change
• Documentation is updated as necessary

Why this change is needed?

There is no way of configuring the overlay setting for a Title as the value is hard-coded as 0 or "Above Chart". Borrowed functionality and tests from the Legend class which already had the ability to configure the overlay setting. Now it can also be set as 1 or "Centered Overlay". Default is still 0.

Couldn't see any documentation to update - if there is, please point me to it.

[oleibman]

What you have is in pretty good shape, but not quite complete. You can set the overlay, and you can write it to a spreadsheet, but you haven't added support for reading it from a spreadsheet. Also, please add the PR number to the change log.

[u01jmg3]

Thanks for the pointers, that should be it

[oleibman]

Getting closer. Now what we still need is a test to demonstrate that Reader and Writer are working correctly. You should be able to model your test after RoundedCornersTest. I don't think you need to create a new sample for this, but you will either need to add one or two spreadsheets in tests/data (2 charts on a single spreadsheet or 1 chart on each of 2 spreadsheets, one with overlay and one without), or just write the code to populate the charts in your test without adding a test spreadsheet.

[u01jmg3]

Is this what you were looking for? I chose not to create a new spreadsheet but to use an existing one (32readwriteAreaChart1.xlsx) as all I needed was one chart to have its title overlay set. Unless you want there to be separation (i.e. don't use a sample file that's used by another test)?

I also chose to add the additional tests you requested to TitleTest.php. Again, unless this is incorrect and you want there to be separation?

There's also no coverage for the same in the Legend model - do you want me to add a test for it?

[oleibman]

I am now a little confused. I agree that the spreadsheet which you changed (32readwriteAreaChart1) has the title overlay set to 1, and the one which you didn't change (32readwriteAreaChart2) has it set to 0. So your test is exactly what I want, but ... the title placement looks exactly the same for both. Do you see a difference? If so, what version of Excel are you using? Can you show me what the difference is?

[u01jmg3]

I too could see no difference when modifying 32readwriteAreaChart1.xlsx (even though the overlay setting is present).

It's easier to see in this spreadsheet: chart-title-overlay.xlsx.

[oleibman]

I agree that your new spreadsheet shows the difference better. So ... please restore 32readwriteAreaChart1 to its original, and add title-overlay.xlsx to tests/data and use it in your test instead of the two 32readwrite charts.

Also, thank you for volunteering to add a similar test for Legend. Please do so.

[u01jmg3]

🤞🏻 that should be it

[oleibman]

Thanks. It looks good now. I will need a little more time to study it, but will probably be able to merge it tonight.

[oleibman]

Thank you for your contribution.