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

Display units in distribution PDF as needed #4610

Merged
merged 4 commits into from
Oct 14, 2024
Merged

Conversation

awwaiid
Copy link
Collaborator

@awwaiid awwaiid commented Aug 25, 2024

Implements #4404

I didn't end up trying to adjust column widths manually, just let it auto-fit.

2024-08-24T23:53:58,059079908-04:00

@awwaiid awwaiid requested review from dorner and cielf August 25, 2024 04:06
@awwaiid awwaiid marked this pull request as ready for review August 25, 2024 04:07
@awwaiid awwaiid added this to the Request Units (Packs) milestone Aug 25, 2024
Copy link
Collaborator

@cielf cielf left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@dorner dorner left a comment

Choose a reason for hiding this comment

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

Minor suggestion, otherwise looks good.

Comment on lines 195 to 199
request_qty = if Flipper.enabled?(:enable_packs) && request_item.unit
"#{request_item.quantity} #{request_item.unit.pluralize(request_item.quantity)}"
else
request_item.quantity || ""
end
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we extract this into a method since it's doing the same thing twice?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@awwaiid awwaiid requested a review from dorner October 13, 2024 13:49
@dorner dorner merged commit 1b1e251 into main Oct 14, 2024
22 checks passed
@dorner dorner deleted the 4404-dist-print-units branch October 14, 2024 23:12
Copy link
Contributor

@awwaiid: Your PR Display units in distribution PDF as needed is part of today's Human Essentials production release: 2024.10.20.
Thank you very much for your contribution!

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.

[PACKS] # 9 Distribution printout for banks includes custom request units, if applicable
3 participants