Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
(feat) O3-3521: Add configurable ability to print multiple stickers on the same page #1934
base: main
Are you sure you want to change the base?
(feat) O3-3521: Add configurable ability to print multiple stickers on the same page #1934
Changes from all commits
d002631
7be590e
164de50
948b38e
8fb8c70
496e474
6f61e2a
9721a2d
9c97742
f77d185
784183e
2f639a1
7edc7b6
eadf109
c2bc395
e182509
0ffcb30
df959ff
5d26a2e
076b51d
e2130b4
ba91b6b
2f9d384
303141a
b951921
5d6b141
aa84553
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I sort of feel that columns per page makes sense, but rows per page should be calculated based on number of columns, size of each sticker, and size of the page
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with you @ibacher. I also wanted to implement it that way.
The main reason why i chose to use this alternative is to give a user control of where they want to insert a page-break so that sticker content doesn't overflow to the next page. For example if a user specifies 2 rows per page, the page break with be inserted after the second row.
Below are the reasons why i resorted to this.
Creating a function that could cater for all the use-cases was seemingly complex compared to merely allow a user to explore what works for him/her.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am happy to fix this in a future PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's discuss this a bit further. In my view, the main case for printing labels is generally not to print them on standard pieces of paper, but I may be wrong about that. Usually, I'd expect we want to print literal stickers on a label printer, in which case, in would be roughly one label per "page", with each page size being the size of the sticker.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, FWIW, the allowed values here are extremely limited. See MDN or the formal spec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ibacher this serves one use-case. If its a thermal printer, then yes we are printing one label for page but with regular printers, we shall be printing multiple labels per page
You can also follow this slack thread about the requirement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have created a ticket to follow this up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While challenging, for printing purposes, I think the natural units to use here are actually inches or centimetres. Both pixels and rems are screen-display units where as printing-wise, we want to express the overall size of a label on a page of paper.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
auto
is fine