Skip to content

Conversation

@campersau
Copy link
Contributor

Because:

  1. Upstream PDFMargin uses string | number
  2. PDFOptions.Width / PDFOptions.Height also use string | number upstream
  3. PdfOptions.Width / PdfOptions.Height using object
    /// <summary>
    /// Paper width, accepts values labeled with units.
    /// </summary>
    public object Width { get; set; }
    /// <summary>
    /// Paper height, accepts values labeled with units.
    /// </summary>
    public object Height { get; set; }
  4. All of these properties were already going to ConvertPrintParameterToInches
    else
    {
    if (options.Width != null)
    {
    paperWidth = ConvertPrintParameterToInches(options.Width);
    }
    if (options.Height != null)
    {
    paperHeight = ConvertPrintParameterToInches(options.Height);
    }
    }
    var marginTop = ConvertPrintParameterToInches(options.MarginOptions.Top);
    var marginLeft = ConvertPrintParameterToInches(options.MarginOptions.Left);
    var marginBottom = ConvertPrintParameterToInches(options.MarginOptions.Bottom);
    var marginRight = ConvertPrintParameterToInches(options.MarginOptions.Right);

PdfOptionsShouldBeSerializable didn't actually test Width / Height which meant that serialisation was broken by the migration to System.Text.Json.
I added a simple PrimitiveTypeConverter to support #1001


PdfOptionsShouldWorkWithMarginWithNoUnits didn't actually test the ConvertPrintParameterToInches logic so I have removed it. It was just testing what PdfOptionsShouldBeSerializable already did.


Also made MarginOptions / PaperFormat records.

This is similar to the object Width / Height in PdfOptions and all of them already got converted by ConvertPrintParameterToInches
@kblok kblok merged commit 71ff822 into hardkoded:master Oct 18, 2024
11 of 12 checks passed
@campersau campersau deleted the objectmarginoptions branch October 18, 2024 17:22
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