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

DOCX: padding of element "Label" not implemented #1941

Closed
speckyspooky opened this issue Oct 20, 2024 · 4 comments
Closed

DOCX: padding of element "Label" not implemented #1941

speckyspooky opened this issue Oct 20, 2024 · 4 comments
Assignees
Labels
Enhancement Small change to improve the current supported functionality
Milestone

Comments

@speckyspooky
Copy link
Contributor

The current implementation of the DOCX emitter has no implementation of padding for the BIRT element "Label".
The padding of the label should be added according to margin to set the according indents.
( This enhancement is familiar with #1895)

The difference from "label" to dynamic text will be that no special grid will be used, only the indent will be used.
MS Word doesn't support a real padding.

Additional on it some methods of the existing margin-padding-calculation are doubled with multiple implementations.
I will provide a PR to add the padding-calculation and optimize the methods.

@speckyspooky speckyspooky added the Enhancement Small change to improve the current supported functionality label Oct 20, 2024
@speckyspooky speckyspooky added this to the 4.18 milestone Oct 20, 2024
@speckyspooky speckyspooky self-assigned this Oct 20, 2024
speckyspooky added a commit to speckyspooky/birt that referenced this issue Oct 20, 2024
merks pushed a commit to speckyspooky/birt that referenced this issue Oct 21, 2024
@hvbtup
Copy link
Contributor

hvbtup commented Oct 21, 2024

I'm not sure what you want to achieve.
If I understand correctly, the old version supported margin, but didn not support padding, and with your change, it supports padding, but not fully. It treats padding the same as margin, but the HTML box has distinct values for border, margin and padding for a reason. For example, to where background color extends.

Maybe it would be better to just document that the DOCX emitter doesn't support padding for labels (and plain text dynamic text items?).

@speckyspooky
Copy link
Contributor Author

The current elements of "dynamic text", "text", "data" supports the paddding values.
This is the reason why the special padding-grid will be created to simulate the padding mechanism.
The support of padding values is a good feature but there are different implementation ways.

My target is here to provide the padding based on the indent-mechanism for ms word of the "label"-element.
The main target in future would be to have an option to change the padding calculation in generally to avoid the usage of the padding-tables but to remain the indent-calculation based on the values of margin-& padding.

@hvbtup
Copy link
Contributor

hvbtup commented Oct 21, 2024

I understand and that it will be useful in common cases.

However, it will not fully support the box model (e.g. background color would not work as intentend, border will not work at all, I suppose) and it will change the behavior of existing reports.

I was going to write: "Also, please keep in mind that items may use display: inline instead of the default display: block". But AFAIK this feature doesn't work anyway with the DOCX emitter.

@speckyspooky
Copy link
Contributor Author

The enhancement is added with PR #1942

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Small change to improve the current supported functionality
Projects
None yet
Development

No branches or pull requests

2 participants