Ghostwriter icon indicating copy to clipboard operation
Ghostwriter copied to clipboard

Added Table to reportwriter

Open domwhewell opened this issue 2 years ago • 7 comments

Issue https://github.com/GhostManager/Ghostwriter/issues/231

Description of the Change This PR adds support for HTML tables in reports.

  • The tables will be added with the default table style:
  1. In docx this will be "Table Grid" which does not exist until a table has been added to the docx template (Table can be deleted after the style will remain) a check has been added to the linter with a failed message if this style does not exist in the docx
  2. In pptx default table styles should always exist
  • The table plugin has been added to tinymce

Alternate Designs N/A

Possible Drawbacks N/A

Verification Process Verified by using the included docx and pptx templates

Release Notes Added support for HTML tables in reports

domwhewell avatar Nov 01 '23 12:11 domwhewell

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (6bd74d2) 92.27% compared to head (20fe6dc) 92.26%.

:exclamation: Current head 20fe6dc differs from pull request most recent head b51ae08. Consider uploading reports for the commit b51ae08 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #355      +/-   ##
==========================================
- Coverage   92.27%   92.26%   -0.02%     
==========================================
  Files         255      255              
  Lines       15996    15996              
==========================================
- Hits        14761    14759       -2     
- Misses       1235     1237       +2     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Nov 02 '23 22:11 codecov[bot]

Thanks for submitting this @domwhewell. Is the intent for this to only work with the Table Grid style?

This is a good start for adding support for HTML tables. The parsing works if you add a basic table with TinyMCE, but it will miss rows with some formatting options. The first one I tried was setting the first row as a header row. If every row is not left set to default "Body" type, this line only gets the first group of rows: tbody = tag.contents[1]

If the first row is set to "Header," the first item in tag.contents is the thead element. We'll need to account for that and loop over more than the first item in tag.contents.

I also tried some other formatting options. I noticed merged cells are not merged once added to the report. A merged cell looks like this in HTML:

<td style="height: 16.5px; width: 50.0317%;" colspan="2">Merge Cells</td>

This will need some handling for merged cells. If the colspan element is set, the next X cells should be merged. Merged cells may not come up much, but the feature should support as much of whatever TinyMCE is capable of unless something just won't translate easily to OXML.

chrismaddalena avatar Nov 27 '23 23:11 chrismaddalena

Hi @chrismaddalena, Yes the intent is only to use the Table Grid style for both word and powerpoint mainly because it should already be there, like the other existing paragraph styles a user could modify the Table Grid style how they would like in the template. Ah I recently came accross that bug where the table would not be added if it was in another index of tag.contents and forgot to commit a change which just looped over that.

domwhewell avatar Nov 28 '23 10:11 domwhewell

Is support for tables going to be fully implemented into GhostWriter/how far off is this from being the complete solution?

secure-sauce-controller avatar Dec 19 '23 17:12 secure-sauce-controller

Hi @secure-sauce-controller, Sounds like this is on the road map for v4.1. The changes currently will add a table in the "Table Grid" style, merge cells in the y or x direction and format any html content within those cells.

Setting the height and width of the cells is not added in this change due to https://github.com/python-openxml/python-docx/issues/247.

domwhewell avatar Dec 20 '23 15:12 domwhewell

Thanks for updating the PR @domwhewell! Now that we're back from the holiday break, we'll take a look at the changes.

And to confirm, this is planned for v4.1, which is planned for later this month. It could slip into February if we identify some sort of problem, but it'll be soon ™️.

chrismaddalena avatar Jan 02 '24 22:01 chrismaddalena

We completed a major refactoring of the entire reportwriter package, so we've manually ported this PR into the v4.1-dev branch. It's working well there, so it will be part of the release! Here is what one of the test table looks like from my testing with formatting options and merged cells. image

chrismaddalena avatar Feb 15 '24 22:02 chrismaddalena