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

feat: post refactoring invoice ui #335

Merged
merged 4 commits into from
Nov 2, 2024
Merged

Conversation

JustSamuel
Copy link
Contributor

Description

Now the UI adheres to the new setup: the user is able to change the amount of the invoice transfer and is warned if these do not match. Translations are left until the translation PR is merge.

image

Related issues/external references

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

@SuperVK
Copy link
Member

SuperVK commented Sep 9, 2024

Will this also include creating invoices? I heard from Robin that #304 will be closed without merging.

@JustSamuel JustSamuel force-pushed the refactor/invoice-rework branch 2 times, most recently from 234b052 to bd0dfd9 Compare September 12, 2024 09:46
@JustSamuel
Copy link
Contributor Author

@SuperVK id prefer to do invoice creation in a different PR to reduce scope.

@CodeNamedRobin
Copy link
Contributor

@JustSamuel is this ready for review? Also for creating invoices?

@JustSamuel JustSamuel marked this pull request as draft September 25, 2024 16:03
@JustSamuel JustSamuel force-pushed the refactor/invoice-rework branch 2 times, most recently from 9ca3afa to a631298 Compare October 14, 2024 18:20
@JustSamuel JustSamuel force-pushed the refactor/invoice-rework branch 6 times, most recently from 7bcfdc6 to cfa5987 Compare October 29, 2024 17:58
@JustSamuel
Copy link
Contributor Author

I have had the beautiful opertunity to test my own code whilst making ALL the intro invoices, I also let the BACPM try it and the first impressions were "yoah dat lijkt wel te werken ofnie", so I think this is fine to merge so that I can go do something else with my life.

@SuperVK please review, I will do the translations when I Get rid of this hangover.

@JustSamuel JustSamuel marked this pull request as ready for review October 29, 2024 21:50
@JustSamuel JustSamuel force-pushed the refactor/invoice-rework branch 4 times, most recently from e75e33d to ebfb097 Compare October 30, 2024 17:55
@SuperVK
Copy link
Member

SuperVK commented Oct 31, 2024

image
I think the date selection could have some improvement, cause it was not clear for me if I have 28 or 29, or both, or none selected. Either something with an outline of green on the dates that have transactions, or in a text box which dates are selected?

@JustSamuel JustSamuel force-pushed the refactor/invoice-rework branch 2 times, most recently from 926752f to 6216e3d Compare November 2, 2024 11:42
@JustSamuel
Copy link
Contributor Author

@SuperVK
image

@JustSamuel JustSamuel force-pushed the refactor/invoice-rework branch from 6216e3d to 834deda Compare November 2, 2024 11:54
@SuperVK
Copy link
Member

SuperVK commented Nov 2, 2024

Nice, I dont think I have much comments right now besides the translations obv

@JustSamuel JustSamuel force-pushed the refactor/invoice-rework branch from 834deda to 7b5e718 Compare November 2, 2024 12:11
@JustSamuel
Copy link
Contributor Author

Nice, I dont think I have much comments right now besides the translations obv

translations have been fixed

@JustSamuel JustSamuel requested review from SuperVK and removed request for CodeNamedRobin November 2, 2024 12:13
@JustSamuel JustSamuel force-pushed the refactor/invoice-rework branch from 7b5e718 to 74f6863 Compare November 2, 2024 12:18
@SuperVK
Copy link
Member

SuperVK commented Nov 2, 2024

image
Missed translation in the toast

image
If not too much trouble, is it possible to make the clear also reset all the other values? like selected point of sales, and how much money is selected, cause that just stays and doesn't clear with the button.

Copy link
Member

@SuperVK SuperVK left a comment

Choose a reason for hiding this comment

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

Lekker gewerkt!

@JustSamuel
Copy link
Contributor Author

image Missed translation in the toast

image If not too much trouble, is it possible to make the clear also reset all the other values? like selected point of sales, and how much money is selected, cause that just stays and doesn't clear with the button.

kinda works, but you need to reselect the user since the two way binding is not that nice atm. (however this will be quite a difficult fix.)

@JustSamuel JustSamuel force-pushed the refactor/invoice-rework branch from 74f6863 to 59e6d2b Compare November 2, 2024 12:49
@JustSamuel JustSamuel force-pushed the refactor/invoice-rework branch from 59e6d2b to 45274dc Compare November 2, 2024 12:49
@JustSamuel JustSamuel merged commit 799b1c8 into develop Nov 2, 2024
3 checks passed
@JustSamuel JustSamuel deleted the refactor/invoice-rework branch November 2, 2024 12:51
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