barryvdh / laravel-dompdf

A DOMPDF Wrapper for Laravel
MIT License
6.74k stars 971 forks source link

Inherit PaperOrientation from config #882

Closed jaap closed 2 years ago

jaap commented 2 years ago

It seems like the paper orientation does not load from the config. Hopefully this pull-request either helps to fix it, prove me wrong or give some direction to @barryvdh to find a swift solution.

erikn69 commented 2 years ago

Maybe better after here https://github.com/barryvdh/laravel-dompdf/blob/e49e1747d71f26454d3138d36247e9f8adc724e7/src/ServiceProvider.php#L50-L53

$dompdf->setPaper(
    $options['default_paper_size'] ?? 'a4',
    $app['config']->get('dompdf.orientation')
);
systemsolutionweb commented 2 years ago

Looks great to me 👍

erikn69 commented 2 years ago

@jaap there is a problem, there are two keys for options, dompdf.defines and dompdf.options https://github.com/barryvdh/laravel-dompdf/blob/e49e1747d71f26454d3138d36247e9f8adc724e7/src/ServiceProvider.php#L31 https://github.com/barryvdh/laravel-dompdf/blob/e49e1747d71f26454d3138d36247e9f8adc724e7/src/ServiceProvider.php#L44 And defines could have a prefix DOMPDF_ https://github.com/barryvdh/laravel-dompdf/blob/e49e1747d71f26454d3138d36247e9f8adc724e7/src/ServiceProvider.php#L40-L41 So, you can't use dompdf.defines.default_paper_size, for compatibility better try this

$dompdf->setPaper(
    $options['default_paper_size'] ?? 'a4',
    $app['config']->get('dompdf.orientation')
);
jaap commented 2 years ago

thanks! @erikn69 , done.

barryvdh commented 2 years ago

Sorry, really busy ATM. Not sure why it isn't working, but I'll look into it.

jaap commented 2 years ago

No worries. I'll fix the merge conflict tomorrow.

PaolaRuby commented 2 years ago

Next time make a rebase instead of a merge

barryvdh commented 2 years ago

I'm not really sure why this option is there. It should just use default_paper_orientation. Changed in https://github.com/barryvdh/laravel-dompdf/commit/a3772cbcd50ec386298958200ce98e4ad3bb355e