-
Notifications
You must be signed in to change notification settings - Fork 195
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
Emit cellcolor and/or textcolor in X(HT)ML #503
Emit cellcolor and/or textcolor in X(HT)ML #503
Conversation
dae5801
to
bbeddfd
Compare
@aardappel This is ready to go from my side. Could you review? |
src/cell.h
Outdated
@@ -260,9 +260,9 @@ struct Cell { | |||
if (text.stylebits & STYLE_BOLD) style += L"font-weight: bold;"; |
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.
could you make a similar change for uses of stylebits
?
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.
Yes, I can try. 😄
bbeddfd
to
6ff1489
Compare
When `cellcolor` or `textcolor` in a cell differs from the one of the parent cell, emit its value. See aardappel#502 for more details.
Text styles like `font-weight`, `font-style` and `font-family` are automatically inherited in CSS. This commit makes the HTML export code only emit these styles when - the text style of the cell changes in comparison to the parent cell or - there is no parent cell. To not disturb the inheritance, this commit also removes the default CSS text styles from the header section.
6ff1489
to
ff701c5
Compare
@aardappel |
@@ -879,8 +879,7 @@ struct Document { | |||
L"<html>\n<head>\n<style>\n" | |||
L"body { font-family: sans-serif; }\n" | |||
L"table, th, td { border: 1px solid #A0A0A0; border-collapse: collapse;" | |||
L" padding: 3px; vertical-align: top; font-weight: normal;" | |||
L" font-style: normal; font-family: sans-serif; }\n" |
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.
why was this deleted?
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.
This is how I got it: Because if we do inheritance and we have parent and child cell share e.g. same font-style that is not the default one (e.g. bold instead of normal), then the code will not add that font-style again for the child as it shall be inherited (this is the goal). But because no font-style is supplied in the child's style attribute then but a default one is defined in the style section in the header, that one is prioritized over the inherited one. In order to avoid that situation, do not create this overriding styles in the header.
@@ -237,12 +237,12 @@ struct Cell { | |||
str.Prepend(wxString() << text.stylebits); | |||
str.Prepend(L" stylebits=\""); |
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.
please also check for these stylebits
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.
Ok please see last commit. It XORs the stylebits from child and parent. Only if they differ, stylebits will be emitted in XML export.
src/cell.h
Outdated
if (text.stylebits & STYLE_ITALIC) style += L"font-style: italic;"; | ||
if (text.stylebits & STYLE_FIXED) style += L"font-family: monospace;"; | ||
if (cellcolor != doc->Background()) | ||
if (parent ? (text.stylebits & STYLE_BOLD) != (parent->text.stylebits & STYLE_BOLD) : true) |
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.
can be simplified to !parent || (text.stylebits & STYLE_BOLD) != (parent->text.stylebits & STYLE_BOLD)
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 thanks!
Style inheritance should be used for the HTML export of the entire document but not for the HTML export of the selection to the clipboard. This commit adds the capability for both ways.
@aardappel So I added two commits. They are good to go for a next review round from my side. |
LGTM! |
Thanks for your highly valuable review and your time spent! I highly appreciate this. I wish you a wunderful weekend! |
No description provided.