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

Add possibility to show scissors instead of text if printable=false #258

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

tafel
Copy link

@tafel tafel commented Aug 28, 2024

This PR adds the opportunity to replace the text "Separate before paying in" with scissors icons.
It addresses issue #180 and #212

// example with TCPDF
$tcPdf = 'an instance of TCPDF';
$output = new QrBill\PaymentPart\Output\TcPdfOutput\TcPdfOutput($qrBill, 'en', $tcPdf);
$output
    ->setPrintable(false)
    ->setScissors(true) // TRUE to show scissors instead of text
    ->getPaymentPart();

The printable parameter take precedence over scissors. Thus, if scissors=true but also printable=true, no scissors are printed. No line is visible.

Tests
All tests are ok. New PDF and HTML files are created so we can see the final result. These are not commited in this PR.

About TCPDF
This implementation should be fully compatible with TCPDF, withtout any external libraries.

About HTML
The CSS code works on chrome browsers. Not sure if it's 100% compatible with all browsers (use :before and :after pseudo-element, and transform property)

About FPDF
Dashed line and text rotation are not supported by default. It seems the only way to add these functionalities is to extends FPDF and add methods (to access protected props and methods, like $this->k or $this->_out()).

Scissors still appear, but lines are not dashed, and the vertical scissors are not rotated.

@kohlerdominik
Copy link
Contributor

This looks solid. Good work.

Are the fonts you used for the _pdfOutputs common across production systems (ubuntu/debian, alpine, probably windows?)

@tafel
Copy link
Author

tafel commented Aug 29, 2024

Thanks @kohlerdominik

All fonts (freeserif for TCPDF, zapfdingbats for FPDF, unicode char for HTML) are all bundled into their respective package/browser.

There are three unknown factors, which are:

  1. how does it react on Windows and Mac? I use Linux, and I made my tests within a docker container (php:8.2-cli)
  2. I'm not a CSS expert. So the HTML part may not be compatible with some browsers (tested only in Chrome and Firefox under ubuntu 22)
  3. does the HTML unicode char display correctly with any <meta charset="" /> (yes it does, now with CSS unicode code)

About FPDF (again)
To address the partial support with FPDF, I had the idea of creating a Trait, bundled into the php swiss qr bill code, which could be used to add the missing methods into the FPDF object.

I added a commit with this proposition

@tafel
Copy link
Author

tafel commented Aug 29, 2024

Regarding #212 , the way layout options are managed seems not optimal (I added a scissors property directly into AbstractOutput class).

Maybe we could identify all possible options, and create a new class (\Sprain\SwissQrBill\PaymentPart\Output\Layout ?) to manage them?

From what I saw, here are some (or all?) options:

option name type values default comment
printable bool true , false false
line string none , solid , dashed solid May not be setted by consumer. The scissors and printable states would manage this
scissors bool true , false false
verticalScissorsPosition string top , bottom top
font string helvetica , frutiger helvetica Maybe the font could be totally custom? In that case, does it violate the QR standard?
textDownArrow bool true , false false

Anyway, all these params could be set on the layout class. And then, this layout can be used to interact with outputs.

Any thoughts about this?

@sprain
Copy link
Owner

sprain commented Aug 30, 2024

Thanks guys, you are awesome!
Will look into this asap. Busy times …

@kohlerdominik
Copy link
Contributor

kohlerdominik commented Aug 30, 2024

@tafel you might want to look into the tests. They should be all green.

Another input from my side, maybe @sprain has some opinion on this as well:
I don't think the property should be called scissors. What about separaterSymbol? Or maybe there is a specific title for it in the (english) specs?

Edit: about your suggestion from the layout - this is really something that sprain has to decide. In my opinion, the best modern approach nowadays is a dedicated options-object (probably readonly, but as long as cloneWith is not an option in php, I dislike readonly). However, less forward facing devs might have issues understanding the pattern.

enum LineType {/* ... */}
enum SeparatorSymbol {/* ... */}
enum SeparatorSymbolPosition {/* ... */}
enum Font {/* ... */}


class LayoutOptions
{
  public bool $printable = false;
  public LineType $line = LineType::solid;
  public SeparatorSymbol $separatorSymbol = SeparatorSymbol::scissors;
  public SeparatorSymbolPosition $separatorSymbolPosition = SeparatorSymbolPosition::top
  public string|Font $font = Font::helvetica;
  public bool $textDownArrow = false;
}

@tafel
Copy link
Author

tafel commented Aug 30, 2024

Here is what I came with. It's quite the same idea as you (it's only pseudo-code). I agree that @sprain must be the referee ;)

<?php declare(strict_types=1);

namespace Sprain\SwissQrBill\PaymentPart\Output;

enum LineStyles
{
    case SOLID;
    case DASHED;
}

enum VerticalSeparatorSymbolPositions
{
    case TOP;
    case BOTTOM;
}

enum Fonts
{
    case DEFAULT;
    case UTF8;
}

final class PrintOptions
{
    private bool $printable;
    private bool $separatorSymbol;
    private VerticalSeparatorSymbolPositions $verticalSeparatorSymbolPosition;
    private bool $textDownArrows;
    private LineStyles $lineStyle;
    private bool $text;
    private string|Fonts $font;

    public function __construct()
    {
        // set default values. Should not generate breaking changes with existing codes
        $this->setSeparatorSymbol(false);
        $this->setPrintable(false);
        $this->setVerticalSeparatorSymbolPosition(VerticalSeparatorSymbolPositions::TOP);
        $this->setFont(Fonts::DEFAULT);
        $this->setTextDownArrows(false);
        $this->setText(true);
    }

    /**
     * Define printable state. If true, nothing is displayed (no line, no text)
     */
    public function isPrintable(): bool;
    public function setPrintable(bool $value): static;

    /**
     * Define scissors state. If true, lines will be dashed, and will hwve scissors
     */
    public function hasSeparatorSymbol(): bool;
    public function setSeparatorSymbol(bool $value): static;

    /**
     * Define down arrows next to "detach" text. If true, will print 3 arrows on right and left of the text
     */
    public function hasTextDownArrows(): bool;
    public function setTextDownArrows(bool $value): static;

    /**
     * Define "detach" text state. If false, no text is displayed
     */
    public function hasText(): bool;
    public function setText(bool $value): static;

    /**
     * Vertical scissors can be on top of the payment part, on at bottom
     */
    public function getVerticalSeparatorSymbolPosition(): VerticalSeparatorSymbolPositions;
    public function setVerticalSeparatorSymbolPosition(VerticalSeparatorSymbolPositions $value): static;

    /**
     * Font usage. Either "default", or "utf8" if one needs to print special chars.
     * Problem with FPDF: true utf8 font is not bundled. We need to authorize custom string, so one can
     * load his own font
     */
    public function getFont(): string|Fonts;
    public function setFont(string|Fonts $value): static;

    /**
     * Line style. Either dashed or solid
     */
    public function getLineStyle(): LineStyles;
}

And how we can use it:

        $output = (new TcPdfOutput($qrBill, 'en', $tcPdf));
        $output
            ->setPrintOptions((new PrintOptions())->setPrintable(false)->setFont(Fonts::UTF8))
            ->setQrCodeImageFormat(QrCode::FILE_FORMAT_SVG)
            ->getPaymentPart();

About tests, I was not sure if I could commit all the new generated PDF documents. I can commit them all, for sure, if it's better.

EDIT: changed suggestion of class definition, based on feedbacks

@tafel
Copy link
Author

tafel commented Aug 30, 2024

@kohlerdominik I see the point of using options objects to manage all these states. But we need to keep in mind that not all combination should be possible.

Is it spec-compatible to have a separator-symbol on solid lines? Can we have the detach text, while printable is true (no line visible)?

If the spec doesn't say anything about that, we could let users configure all the props individually. But if we need to make checks, I'm more for setters which reequilibrate values between props, either by changing them, or by throwing exceptions.

About the separator symbol, I agree to use this name. But I think it's better to only have a bool, since it's not useful to let user choose a custom separator symbol. Moreover, this symbol is not the same across outputs.

PS: just as a reminder about fonts. Directly from SIX specification

Only the sans-serif fonts Arial, Frutiger, Helvetica and Liberation Sans are permitted in black

Which is problematic because none of them has special chars like ł or ą. For TCPDF, I use FreeSans , and for FPDF, I didn't find a way to load gracefully a suitable font until now...

@tafel
Copy link
Author

tafel commented Sep 26, 2024

I read a bit of documentation, to see what are the official options available. It seems there're only three:

  1. no line at all
  2. solid lines and text (only text without down arrows)
  3. dotted lines, with scissors symbol (no other symbol authorized)

See chapter 3.7 (https://www.six-group.com/dam/download/banking-services/standardization/qr-bill/ig-qr-bill-v2.3-en.pdf)

If the QR-bill with payment part and receipt or the separate payment part with receipt are generated
as a PDF document and sent electronically, the A6 format of the payment part and the receipt on the
left must be indicated by lines. Each of these lines must bear the scissors symbol or alternatively the
instruction "Separate before paying in" above the line (outside the payment part)
. This indicates to the
debtor that he or she must neatly separate the payment part and receipt if they want to forward the
QR-bill to their financial institution by post for payment, or settle it at the post office counter (post
office branches or branches of partner organizations).

So the available options should be:

  1. printable or not
  2. with text (and solid lines) or with scissors (and dotted lines)

Your thoughts?

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.

3 participants