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 various dl-horizontal views #840

Merged
merged 6 commits into from
Mar 24, 2021

Conversation

twothreenine
Copy link
Contributor

@twothreenine twothreenine commented Feb 25, 2021

I raised the width of dl-horizontal titles (used on pages like suppliers/show, group_orders/show, and here finance/invoices/show as well), as some titles, esp. in German, were cut off before. I'm not sure if this could have negative effects on small screens like on mobile devices. I guess there has been a reason this wasn't changed.
Also I added some 'line spacing' via margin-bottom to increase the readability of those pages.

With these changes, we could also use dl-horizontal for the invoices/show page, which would again improve readability and make it easier to compare dates, amounts, deposit etc.

Rework of the page of a single invoice
- using dl for better readability
- formatting dates (created_at without time info, or is that important?)
- formatting note to support line breaks
- hide attachment line if none attached
supporting line breaks
Copy link
Member

@paroga paroga left a comment

Choose a reason for hiding this comment

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

small issue, otherwise good to go. thanks!

- sum = order.sum
- total += sum
= ', ' if index > 0
= link_to format_date(order.ends), new_finance_order_path(order_id: order) #, style: "margin-right: 0.2em;"
Copy link
Member

Choose a reason for hiding this comment

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

can you please remove the unnecessary comment

@twothreenine
Copy link
Contributor Author

Just a note for the foodcoopsat fork: Please be aware that this collides with this commit (number of indents has to aligned)

@paroga
Copy link
Member

paroga commented Mar 24, 2021

sorry I missed the change for the created_at in my last review: Is there a reason why you changed it to date-only? Sometimes it's quite interesting when members created invoices. E.g. sometimes I wondered why I missed to pay an invoice and then saw that it has just been created a few minutes after paying all unpaid ones.

@twothreenine
Copy link
Contributor Author

The date was not formatted with format_time. When you switch to English, you still see the German representation, unlike the other dates in the invoice, which would be confusing when you are trying to compare them.
format_datetime gives a very long representation like Tue, 16 Mar 2021 22:27:55 +0000 and I thought the time is unnecessary anyway.
We could add a short version of format_datetime which just returns the date as in format_date + hh:mm

@paroga
Copy link
Member

paroga commented Mar 24, 2021

we can change format_time in ApplicationHelper to provide a localized representation, but i'd consider that an independent issue

@paroga paroga merged commit b2a6c19 into foodcoops:master Mar 24, 2021
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