Skip to content

update: body height limit #18

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.

Sign up for GitHub

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

Closed
dvoiejanovic wants to merge 1 commit into from
Closed

update: body height limit #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/utils/layout/calculate-page-layout.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,12 @@ export default function calculatePageLayout(
const footerY = footerHeight ? -1 : 0;
const bodyY = footerHeight - (pageHeight ? 1 : 0);
const backgroundY = 0;
const BODY_HEIGHT_MIN_FACTOR = 0.25;
Copy link
Copy Markdown
Collaborator

@trogulja trogulja Mar 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jako nisam fan rjesavanja lokalnih problema na globalnoj razini. To uvijek, ali bas uvijek vodi u krivom smjeru i onda hrpa bugova ispliva.

Ovo nikako ne bi rjesavao hardcodiranjem, pogotovo ne u libu - jer to vodi prema panicnom republishanju paketa da se pofixaju neki drugi problemi koji ce tek isplivati.

Ovo trebamo rjesavat na nacin na genericki nacin - da se otvori konfiguracijska opcija overrideanja tog faktora, pa onda overrideaj na exporteru prilikom inicijalizacije. Pa ako se pokaze da je override los, puno ce se brze moc reagirat uz daleko manji friction.


if (bodyHeight < pageHeight / 3) {
throw new Error(
`Header/footer too big. Page height: ${pageHeight}px, header: ${headerHeight}px, footer: ${footerHeight}px, body: ${bodyHeight}px.`
);
if (bodyHeight < pageHeight * BODY_HEIGHT_MIN_FACTOR) {
throw new Error('The header and footer are too large to fit on the page. Please reduce their size and try again.', {
Copy link
Copy Markdown
Contributor Author

@dvoiejanovic dvoiejanovic Mar 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Želim ovu poruku prikazati na Exporteru u nekakvom error templateu, pa sam je učinio više user-friendly.

Copy link
Copy Markdown
Collaborator

@trogulja trogulja Mar 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upravo je ovo dio na koji sam mislio kad sam rekao na tasku da error handling ne bi trebali raditi u libu.

Lib je iskljucivo developer facing stvar -> errori koje ovdje throwamo moraju imati informacije korisne za developere, ne za usere. Ovim gubimo cijeli reasoning zasto je throwano i sto se desilo sa velicinama hadera / footera / bodya.

Ono sto bi trebalo napraviti je na exporteru handleati ovdje throwane errore u catch bloku:

// handle-export-document.ts linija ~98
if (Error.isError(error) && error.message.startsWith('Header/footer too big.') {
  return 'The header and footer are too large to fit on the page...'
}

i onda taj kod handleati unutar rute da se returna kako spada.

cause: `Page height: ${pageHeight}px, header: ${headerHeight}px, footer: ${footerHeight}px, body: ${bodyHeight}px`,
});
}

return {
Expand Down
2 changes: 1 addition & 1 deletion test/utils/layout/calculate-page-layout.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ describe('calculatePageLayout', () => {
const pageHeight = 1000;

expect(() => calculatePageLayout(settings, pageHeight)).toThrow(
`Header/footer too big. Page height: 1000px, header: 1000px, footer: 1000px, body: -998px.`
'The header and footer are too large to fit on the page. Please reduce their size and try again.'
);
});

Expand Down