Skip to content
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

Support rotate attribute on table entries #2717 #3161

Merged
merged 5 commits into from Feb 14, 2019

Conversation

Projects
5 participants
@robander
Copy link
Member

commented Nov 28, 2018

Implements #2717 -- adds PDF support for entry/@rotate (added in DITA 1.3) using the container method suggested by @raducoravu in that issue.

I've tested the result with Antenna House and it works well. FOP gave bad results -- the text was rotated, but the cell kept its original dimensions, so that the rotated text writes over surrounding cells. I've updated the FOP table override to skip adding the container. I'll have the XEP test completed soon; if it works or if the rotation is ignored, I expect to keep the code as-is; if the output is broken as in FOP, we should update the org.dita.pdf2.xep plugin to skip rotation here.

As part of this, I've moved where <row> based flagging is processed within the entry and put it into the processEntryContent named template. This seems like an acceptable spot because any row flag has to be processed as entry content; it also simplifies the code, allowing us to call the ancestor-start-flag and ancestor-end-flag modes once each rather than 4 times each. Provided we keep this change, any DITA-OT 3.2 migration instructions may want to mention the move in case people have custom processing for entry elements.

robander added some commits Nov 27, 2018

Support rotate att on table entry #2717
Signed-off-by: Robert D Anderson <robander@us.ibm.com>
Avoid entry rotation for FOP #2717
Signed-off-by: Robert D Anderson <robander@us.ibm.com>

@robander robander added this to In progress in 3.3 via automation Nov 28, 2018

@raducoravu

This comment has been minimized.

Copy link
Member

commented Nov 28, 2018

@robander I took a peek at what XSL-FO our Chemistry processor generates for a DITA table with a rotated cell and it's something like this:

            <fo:table-cell>
                <fo:block>
                    <fo:block-container reference-orientation="90"
                        width="150px" height="80px" margin-top="0"
                        margin-bottom="0">
                        <fo:block-container reference-orientation="0"
                            margin="0" height="80px" padding-bottom="0"
                            padding-left="0" padding-right="0" padding-top="0"
                            text-align="left"
                            keep-together.within-line="always"
                            wrap-option="no-wrap" width="150px">
                            <fo:block end-indent="0" start-indent="0"
                                >abcdef</fo:block>
                        </fo:block-container>
                    </fo:block-container>
                </fo:block>
            </fo:table-cell>

so it has two block containers, one inside the other, and this seems to work in Apache FOP.

Skip cell rotation for XEP #2717
Signed-off-by: Robert D Anderson <robander@us.ibm.com>
@robander

This comment has been minimized.

Copy link
Member Author

commented Nov 30, 2018

@raducoravu When I just try adding an extra container (outer one with reference-orientation="90" and inner with reference-orientation="0"), I get the same results of text writing over other cells. I also tried adding the extra <fo:block> outside of the containers, as in your example:

<fo:table-cell text-align="from-table-column()">
 <fo:block>
  <fo:block-container reference-orientation="90">
   <fo:block-container reference-orientation="0">
    <fo:block end-indent="3pt" space-after="3pt" space-after.conditionality="retain" space-before="3pt" space-before.conditionality="retain" start-indent="3pt">testing rotate</fo:block>
   </fo:block-container>
  </fo:block-container>
 </fo:block>
</fo:table-cell>

The containers in your example explicitly set the height / width of the box within the table cell -- it appears that's what makes this work, rather than the extra fo:block-container. When I add height="100px" width="150px" to each container, the cell expands to that size, and the text fits / rotated output looks good. But, I don't think this is something we can automatically calculate or generate without knowing a lot about the table and cell.

@robander

This comment has been minimized.

Copy link
Member Author

commented Nov 30, 2018

Test with XEP correctly rotated the text and it fit within the cell, but even with a small amount of text, the rotation caused the cell to run from the original starting point all the way to the bottom of the page. So, I've created an override to skip the rotating container there as well. I still need to update the XEP shell to include that override, but for now I'm holding off until 3.2.1 is merged to avoid a merge conflict with https://github.com/dita-ot/dita-ot/pull/3159/files#diff-b2fe3bc975f3eb841843b5bb49084abcR14

@kbrown01

This comment has been minimized.

Copy link

commented Dec 1, 2018

@kbrown01

This comment has been minimized.

Copy link

commented Dec 1, 2018

@raducoravu

This comment has been minimized.

Copy link
Member

commented Dec 3, 2018

@robander Looked more at our CSS example of how to rotate table cells:

https://www.oxygenxml.com/doc/versions/20.1/ug-editor/topics/dcpp_tables.html

and it says something like:

The width is required - it will become the height of the cell. If this is not specified, the vertical rotated text will bleed out of the cell.

*[outputclass ~= "rotated"] > * {
  .....
  width:150px; 
  ...
}

so you're right, in order to make this work at least with Apache FOP you need to specify the height of the rotated cell, maybe instead of disabling the rotation support with FOP you could just specify in the XSLTs a default height for the rotated cells and let people override that specified cell height using their own PDF plugins.

@robander

This comment has been minimized.

Copy link
Member Author

commented Dec 3, 2018

@raducoravu we could do that but it makes me nervous - I don't think we have any other cases where the PDF output is done that way (we pick a default size for your text -- it only looks "right" if you have that amount of text, otherwise you have to customize). The customization route would also mean that an author has to make all their rotated cells the same size, unless they get really clever.

A couple of other ideas might be

  • Set a default of 150, but also add support for a custom @outputclass token that can be used to set the cell size. This way someone can fix each cell using the correct size, and can also fix the output without requiring a full customization.
  • Leave instructions in the XSLT override on how to turn on rotated table entries.

@jelovirt or @infotexture , any thoughts? I really don't like the idea of turning on a feature that might often result in bad output without custom XSL code.

@raducoravu

This comment has been minimized.

Copy link
Member

commented Dec 4, 2018

@robander I like both of your suggestions, actually both could be implemented... maybe even issue a warning in the XSLT that a cell was found which needs to be rotated but it does not have the width set on it.

Make custom rotation easier in fop, xep
Signed-off-by: Robert D Anderson <robander@us.ibm.com>
@robander

This comment has been minimized.

Copy link
Member Author

commented Feb 8, 2019

Updated based on discussion at yesterday's DITA-OT contributor call.

There was opposition to adding custom tokens into <entry> attributes in order to support formatters; instead, I've simplified the code to make it easier to customize output for other formatters to set default or custom height values. This also simplifies the code and reduces what we override for FOP/XEP, because the updated mode only reproduces a 3 lines of code for each instance (minus the dropped container). Code is also now commented to indicate how to turn on custom rotation.

@robander robander moved this from In progress to Needs review in 3.3 Feb 8, 2019

<!--
This file is part of the DITA Open Toolkit project.
Copyright 2016 IBM Corporation

This comment has been minimized.

Copy link
@jelovirt

jelovirt Feb 9, 2019

Member
Suggested change
Copyright 2016 IBM Corporation
Copyright 2019 IBM Corporation
Fix copyright date in new file
Signed-off-by: Robert D Anderson <robander@us.ibm.com>

3.3 automation moved this from Needs review to Reviewer approved Feb 14, 2019

@jelovirt jelovirt added the plugin/pdf label Feb 14, 2019

@robander robander merged commit cc3b4c2 into develop Feb 14, 2019

4 checks passed

DCO DCO
Details
WIP Ready for review
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

3.3 automation moved this from Reviewer approved to Done Feb 14, 2019

@robander robander deleted the feature/rotateentry branch Feb 14, 2019

@robander robander added this to the 3.3 milestone Feb 19, 2019

@infotexture infotexture added feature and removed enhancement labels Feb 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.