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

Improve readability of Fax PDF (and CSV) #42

Closed
twothreenine opened this issue Jan 18, 2024 · 8 comments
Closed

Improve readability of Fax PDF (and CSV) #42

twothreenine opened this issue Jan 18, 2024 · 8 comments
Assignees
Labels

Comments

@twothreenine
Copy link
Contributor

twothreenine commented Jan 18, 2024

The PDF still looks like this:

(for the weird carrots amount, see #41; also why are the units in German, this is from Foodsoft one -> #38)

grafik

I propose to:

  • omit the Unit quantity column since it's unnecessary (as far as I can see)
  • place the Unit column directly right of the Amount column
  • (low priority) omit .0 in the amount, unless there are decimals other than 0, for example 0.456 (you could also make a case to include .0 for scalar units)

So it would look like this: (except for the .0 still there)

grafik

Reading "3 Kilogramm Oyster mushrooms" is much more intuitive than in the example above.


Same applies to the Fax CSV, although the current order there is different than in the PDF.


In case of the Fax text, the unit column is missing:

  Number   Amount   Name
                1   Bread
                4   Bread rolls
                3   Smoked tofu
                1   Muesli
              235   Carrots
                3   Oyster mushrooms

I don't know if Fax text is used anywhere, though. It doesn't appear in the e-mail sent to the supplier (nor as attachment)

@lentschi lentschi self-assigned this Apr 12, 2024
@lentschi lentschi added this to the Ready for MR milestone Apr 12, 2024
@lentschi
Copy link
Contributor

Fixed, apart from the following points:

(low priority) omit .0 in the amount, unless there are decimals other than 0, for example 0.456 (you could also make a case to include .0 for scalar units)

I don't think is a good idea, as the amounts are aligned right, so if we'd have one amount with decimals and one without, the first non-decimal digit would not be aligned consistently. So I've now changed it to have exactly 2 decimals no matter how many there actually are (e.g. showing .00 if there are none).

In case of the Fax text, the unit column is missing

Yes, and I think that is intentional. The text export obviously has to have hardcoded "column" sizes (currently hardcoded to eight characters). So only the rightmost column has flexible width, which is already taken by "name". The unit column would however also need to have flexible width.
So I left it more or less unchanged. (I only set the decimals for the amount to 2 as well.)

@twothreenine
Copy link
Contributor Author

So I've now changed it to have exactly 2 decimals no matter how many there actually are (e.g. showing .00 if there are none).

I have a problem with that: Amounts like 0.725 kg will be omitted to 0.72 or 0.73. It should work for most use cases with exactly 3 decimals, though.

@lentschi lentschi reopened this Apr 12, 2024
@lentschi
Copy link
Contributor

Okay - changed it to 3 decimals 👍

lentschi added a commit that referenced this issue May 4, 2024
lentschi added a commit that referenced this issue May 4, 2024
Changed decimals to 3 as requested
lentschi added a commit that referenced this issue May 4, 2024
lentschi added a commit that referenced this issue May 4, 2024
Changed decimals to 3 as requested
@twothreenine
Copy link
Contributor Author

(low priority) omit .0 in the amount, unless there are decimals other than 0, for example 0.456 (you could also make a case to include .0 for scalar units)

I don't think is a good idea, as the amounts are aligned right, so if we'd have one amount with decimals and one without, the first non-decimal digit would not be aligned consistently. So I've now changed it to have exactly 2 decimals no matter how many there actually are (e.g. showing .00 if there are none).
[...]
Okay - changed it to 3 decimals 👍

I'm still not really happy with the .000 for piece units:

grafik

I think this would be easier to read, even though it's not aligned:
(using 3 decimals for scalar units, but no decimals for piece units)

grafik

@twothreenine twothreenine reopened this May 7, 2024
lentschi added a commit that referenced this issue May 24, 2024
Changed decimals to 3 as requested
@lentschi lentschi reopened this May 24, 2024
@lentschi
Copy link
Contributor

lentschi commented May 24, 2024

I think this would be easier to read, even though it's not aligned:
(using 3 decimals for scalar units, but no decimals for piece units)

Okay, this is different from what we originally agreed on. But, okay... You mean like this?

But if we don't care about alignment, could we then just always display all the decimals there are?

@twothreenine
Copy link
Contributor Author

Yeah I wasn't happy with it when I looked at the result.

You mean like this?

Looks like I suggested, I'll test it later.

But if we don't care about alignment, could we then just always display all the decimals there are?

I see a case for alignment between amounts like

0.625 kg
0.800 kg

just not between

0.625 kg
   10 Package

however, this would also result in (a) additional decimals being ignored, and (b)

  0.620 kg
550.000 g
  • (a) could be solved by checking if there are more than 3 decimals and including them in that (probably rare) case
  • (b) could be solved by only displaying 3 decimals in case there are any decimals, but at the cost of
0.620 kg
    6 kg

@lentschi
Copy link
Contributor

I am confused as to how you want it to be in the end...? Or are you just brainstorming here?

[...] this would also result in (a) additional decimals being ignored [...]

Is that really a problem? I think we need to consider the use cases leading up to the "fax" export - I wasn't in the project when this was designed - here's what I guess:

Use case no. 1 (and the only one I can think of - do you have more?):
"As a foodsoft user I want to send the list of articles ordered by ordergroups to a supplier for them to be delivered."

Thus, I can't think of a use case, where it would make sense to configure the supplier_order_unit in a way that would require the amount to be ordered to have more than even two decimals - can you? So for your example of 0.625 kg: I believe that either the supplier sells the article in grams (as per their own article lists provided as CSV or whatever) or they would not allow / ignore a granularity of 0.005 kg.

Apart from that - So far, we've only discussed PDF and CSV at the same time. I think however, that for CSV we could just skip the formatting/rounding all together. CSVs are usually either opened in Excel/Calc where you could just choose whichever number format you like or parsed by some other script/app for which it shouldn't make any difference either, right?

twothreenine added a commit that referenced this issue May 27, 2024
- outsource format_units_to_order to OrderHelper
- fax text: include unit, adjust column width
- fax PDF & text: only include order number if any present
twothreenine added a commit that referenced this issue May 27, 2024
- outsource format_units_to_order to OrderHelper
- fax text: include unit, adjust column width
- fax PDF & text: only include order number if any present
@twothreenine
Copy link
Contributor Author

twothreenine commented May 27, 2024

I just wanted to note those potential issues but personally, I'm happy with the solution for the PDF as in your PR,

I noticed that the scale of order_article.units_to_order is limited to 3 decimals anyways, so I opened #72 for the implications.

I think however, that for CSV we could just skip the formatting/rounding all together

We need it for localization and I'd prefer to avoid any irrelevant digits in the CSV, so I used strip_insignificant_zeros: true here.
I added that to the PR along with the following features:

  • outsource format_units_to_order to OrderHelper
  • fax text: include unit, adjust column width
  • fax PDF & text: only include order number column if any present

The text export obviously has to have hardcoded "column" sizes (currently hardcoded to eight characters). So only the rightmost column has flexible width, which is already taken by "name". The unit column would however also need to have flexible width.

I think I found a good solution by determining the length of the longest entry and setting the width to that (also for order number and amount)
Looks like this now:

Amount Unit            Name
     1 Piece (700 g)   Bread
     1 Crate (10 l)    Beer
     1 Piece (160 g)   Smoked tofu
 0.250 kg              Carrots
 0.375 kg              Potatoes
     2 Piece (1.3 kg)  Pumpkin
Number                  Amount Unit           Name
                             1 Piece (700 g)  Bread
very-long-order-number       2 Piece (160 g)  Smoked tofu
                         0.750 kg             Carrots
                        12.625 kg             Potatoes

lentschi added a commit that referenced this issue Jun 21, 2024
- Adapted order_txt to generalized creating the text table and added
  spec
- Code style fixes for order_fax
lentschi added a commit that referenced this issue Jun 28, 2024
lentschi added a commit that referenced this issue Jun 28, 2024
…onvertability (#70)

* On #42: Fax pdf/csv: Decimals dependant on supplier_order_unit's si convertability

* Solve #42: Improve fax PDF, CSV, text

- outsource format_units_to_order to OrderHelper
- fax text: include unit, adjust column width
- fax PDF & text: only include order number if any present

* On #42:

- Adapted order_txt to generalized creating the text table and added
  spec
- Code style fixes for order_fax

* On #42 Fixes error dad0bb9#r143648091

---------

Co-authored-by: twothreenine <leonard_ostler@yahoo.de>
lentschi added a commit that referenced this issue Jul 26, 2024
lentschi added a commit that referenced this issue Jul 26, 2024
Changed decimals to 3 as requested
lentschi added a commit that referenced this issue Jul 26, 2024
…onvertability (#70)

* On #42: Fax pdf/csv: Decimals dependant on supplier_order_unit's si convertability

* Solve #42: Improve fax PDF, CSV, text

- outsource format_units_to_order to OrderHelper
- fax text: include unit, adjust column width
- fax PDF & text: only include order number if any present

* On #42:

- Adapted order_txt to generalized creating the text table and added
  spec
- Code style fixes for order_fax

* On #42 Fixes error dad0bb9#r143648091

---------

Co-authored-by: twothreenine <leonard_ostler@yahoo.de>
lentschi added a commit that referenced this issue Oct 11, 2024
lentschi added a commit that referenced this issue Oct 11, 2024
Changed decimals to 3 as requested
lentschi added a commit that referenced this issue Oct 11, 2024
…onvertability (#70)

* On #42: Fax pdf/csv: Decimals dependant on supplier_order_unit's si convertability

* Solve #42: Improve fax PDF, CSV, text

- outsource format_units_to_order to OrderHelper
- fax text: include unit, adjust column width
- fax PDF & text: only include order number if any present

* On #42:

- Adapted order_txt to generalized creating the text table and added
  spec
- Code style fixes for order_fax

* On #42 Fixes error dad0bb9#r143648091

---------

Co-authored-by: twothreenine <leonard_ostler@yahoo.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants