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

Translates letter #155

Merged
merged 2 commits into from
Sep 4, 2024
Merged

Translates letter #155

merged 2 commits into from
Sep 4, 2024

Conversation

jplot
Copy link
Contributor

@jplot jplot commented Aug 28, 2024

I've implemented i18n to be able to generate the initialization letter in several languages and I've taken the opportunity to review the formatting.
before: EBICS ini (before).pdf
after: EBICS ini.pdf

@jplot jplot force-pushed the translate-letter branch from c502b5b to 2636e96 Compare August 28, 2024 16:40
@jplot jplot force-pushed the translate-letter branch from 2636e96 to ad09384 Compare August 29, 2024 08:45
@jplot jplot marked this pull request as ready for review August 29, 2024 08:53
@tobischo
Copy link
Collaborator

Regarding the formatting change I have the following feedback:

  • The 3 text field format with location, name and signature is common this way (in Germany) and I would keep that. I think the new format makes it less obvious where to put which part.
  • Not sure about the dotted border, but ok I guess. I think I would go for a single thin solid line
  • The vertical and horizoncal centering on the fields at the top look off, especially because the number of fields is uneven. 2 columns is ok, but align it to a top border and group the fields together differently, e.g. date, time, recipient in one column and host id, user id, partner id and version in the other

@jplot
Copy link
Contributor Author

jplot commented Sep 2, 2024

Here's the rendering after modification
EBICS ini.pdf

@tobischo tobischo merged commit b29ba0c into railslove:master Sep 4, 2024
1 check passed
@jplot jplot deleted the translate-letter branch September 5, 2024 07:46
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.

2 participants