-
Notifications
You must be signed in to change notification settings - Fork 556
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
Page size 3value fix #383
Page size 3value fix #383
Conversation
Tested on latest version - works
|
@@ -94,7 +94,7 @@ public List<PropertyDeclaration> buildDeclarations( | |||
} else { | |||
throw new CSSParseException("Value for " + cssName + " must be a length or identifier", -1); | |||
} | |||
} else { /* values.size == 2 */ | |||
} else if (values.size() == 2) { /* values.size == 2 */ |
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.
Is this comment is still needed?
It just duplicates the code.
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.
cleaned, sorry (modification made in plaintext editor)
@@ -147,6 +147,35 @@ public List<PropertyDeclaration> buildDeclarations( | |||
} else { | |||
throw new CSSParseException("Invalid value for size property", -1); | |||
} | |||
} else if (values.size() == 3) { | |||
PropertyValue value1 = (PropertyValue) values.get(0); |
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't we have at least some tests for this code?
It should be easy:
class SizePropertyBuilderTest {
private final SizePropertyBuilder builder = new SizePropertyBuilder();
@Test
void buildDeclarations() {
PropertyValue cssValue = new PropertyValue(CSS_NUMBER, 2.0f, "foo");
List<PropertyDeclaration> result = builder.buildDeclarations(COUNTER_INCREMENT, List.of(cssValue), 42, false);
assertThat(result).hasSize(1);
assertThat(result).containsExactly(new PropertyDeclaration(COUNTER_INCREMENT, cssValue, false, 42));
}
}
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'll try to prepare that, I'm a bit struggling with building the lib in Eclipse...
036ea95
to
652f49d
Compare
@jelinj8 Thanks for the PR. |
I wanted to create a PDF for a label printer with landscape orientation AND a custom size which evidently isn't supported.
This is an attempt to fix that (+ fixed one typo in original code reporting error on incorrect parameter value).
I'm unable to test this properly right now with current version as I'm unable to compile the library with Java 17 (still using FS 9.4.1 with Java 1.8), but it compiled OK and works (with slight modifications for the old version), this is to get it fixed when I'll be able to upgrade and to give something back, thanks.
Succesfully created portrait label on landscape paper 40x20mm...