-
Notifications
You must be signed in to change notification settings - Fork 33
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
Fix ODT exporting tables #274
base: master
Are you sure you want to change the base?
Conversation
It fails in 4 tests and you changed |
It shouldn't be null because null isn't a valid CSS value for margin. In fact, if you see the lines 710-717, you'll see that for the problem you're facing (float is null), the code set the margins to 0cm. I just set it directly to 0cm to avoid code changes, but you're free to change the float detection and variable assignments if you will and commit a PR to the code. |
$margin_right = $css_properties ['margin-right']; | ||
$margin_bottom = $css_properties ['margin-bottom']; | ||
$margin_left = $css_properties ['margin-left']; | ||
$margin_top = $css_properties ['margin-top'] ?? '0cm'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0 does not really need a unit. So '0' instead of '0cm' would be fine (and shorter).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure, because it isn't exactly CSS, it uses ODT that uses a XML like syntax. If you do not specify the unit the ODT rendering on LibreOffice may fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, ok. I didn't realise that. I have no experience with ODT so I'll shut up now ;-)
@@ -692,7 +692,7 @@ function renderODTOpenTable ($renderer, $attr, $style, $attr_string) { | |||
} | |||
$frame_props ['min-height'] = '1cm'; | |||
$frame_props ['width'] = $attr ['width']; | |||
$frame_props ['float'] = $css_properties ['float']; | |||
$frame_props ['float'] = $css_properties ['float'] ?? null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'none'
might be better than null
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It isn't a CSS property, it's for PHP control only. On line https://github.com/selfthinker/dokuwiki_plugin_wrap/blob/master/helper.php#L709 PHP just checks if this value is null to proper set the margin and padding values to '0cm'.
Submited PR #275, but same errors, same tests. |
Fixes #272