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

Page size 3value fix #383

Merged
merged 3 commits into from
Sep 16, 2024

Conversation

jelinj8
Copy link
Contributor

@jelinj8 jelinj8 commented Sep 6, 2024

I wanted to create a PDF for a label printer with landscape orientation AND a custom size which evidently isn't supported.

@page {
   size: 40mm 20mm landscape;
}

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...

@jelinj8
Copy link
Contributor Author

jelinj8 commented Sep 6, 2024

Tested on latest version - works
label.pdf
configured by

@page {
	margin: 0;
	size: 40mm 20mm landscape;
}

@@ -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 */
Copy link
Contributor

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.

Copy link
Contributor Author

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);
Copy link
Contributor

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));
    }
}

Copy link
Contributor Author

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...

@asolntsev asolntsev added this to the 9.9.3 milestone Sep 16, 2024
@asolntsev asolntsev self-assigned this Sep 16, 2024
@asolntsev asolntsev merged commit d7947f3 into flyingsaucerproject:main Sep 16, 2024
4 checks passed
asolntsev added a commit that referenced this pull request Sep 16, 2024
@asolntsev
Copy link
Contributor

@jelinj8 Thanks for the PR.
I've released FS 9.9.3 and added few tests.

@jelinj8 jelinj8 deleted the page_size_3value_fix branch September 19, 2024 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants