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

Pp 623 implement full screen skonto details snapshot from the invoice #622

Conversation

ValentinaIancu-Gini
Copy link
Contributor

@ValentinaIancu-Gini ValentinaIancu-Gini commented Aug 12, 2024

When the user taps that document preview view from the main screen(Skonto screen), the app should display the image of the invoice with the Skonto details highlighted in full-screen mode.

…tion call and error handling into separate private methods

- Extracted the `extractions` call into a new private method `startExtraction`
- Moved error handling into a new private method `handleError`
- Left the completion handler outside of the `handleError` method to maintain separation of concerns
- Improved code modularity and readability

PP-623
…CaptureNetworkService` public protocol

PP-623
…wViewController`

- `didTapInvoicePreview` added
- cleanup in `SkontoVieModel` to follow easily the code
PP-623
…Id: String, number: Int, sizeVariant: Document.Layout.SizeVariant)`

PP-623
…formation into `DocumentPagesViewController`

PP-623
Copy link
Contributor

@mrkulik mrkulik left a comment

Choose a reason for hiding this comment

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

@ValentinaIancu-Gini Great job! I left some suggestions, please let me know your thoughts!

On iPad landscape vertical scroll is not working (Image is displayed outside the frame at start and we can't scroll to the bottom/top), with horizontal all ok. Based on Figma, in landscape mode document should be displayed in full height:
https://github.com/user-attachments/assets/8fe0caca-5deb-42e1-b858-7ddde9c61629
After zooming, vertical scroll works strange, can't scroll to see image details:
https://github.com/user-attachments/assets/75320583-2c87-424d-b728-e5b82bf010ad

…` and `large` cases and remove `SizeVariant` from Layout

PP-623
…n-Skonto-details-snapshot-from-the-invoice

# Conflicts:
#	BankSDK/GiniBankSDK/Sources/GiniBankSDK/Core/Skonto/SkontoCoordinator.swift
#	BankSDK/GiniBankSDK/Sources/GiniBankSDK/Core/Skonto/SkontoViewModel.swift
#	BankSDK/GiniBankSDK/Sources/GiniBankSDK/Resources/GiniImages.swift
Copy link

sonarcloud bot commented Aug 19, 2024

Copy link
Contributor

@mrkulik mrkulik left a comment

Choose a reason for hiding this comment

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

@ValentinaIancu-Gini looks nice!

@ValentinaIancu-Gini ValentinaIancu-Gini merged commit 038951e into PP-Skonto-version-2.0 Aug 19, 2024
9 of 23 checks passed
@ValentinaIancu-Gini ValentinaIancu-Gini deleted the PP-623-Implement-Full-Screen-Skonto-details-snapshot-from-the-invoice branch August 19, 2024 14:20
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