[#172992641] Check bonus eligibility #1841
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…eck-bonus-eligibility
fabriziofff
requested review from
francescopersico,
matteodesanti and
Undermaken
as code owners
Affected stories
Generated by 🚫 dangerJS |
Undermaken
suggested changes
May 25, 2020
| // TODO: link with the right reducer | ||
| isLoading: true, | ||
| // TODO: link with the right reducer | ||
| errorText: "A text that explains why the operation failed" |
There was a problem hiding this comment.
I guess the error could be a network error, otherwise we should receive a response from our backend.
Could it be a solution to show an error toast informing the user that something went wrong?
There was a problem hiding this comment.
If we want to have a generic error, I can remove the errorText and use the default message used within the GenericErrorComponent ("Ooops, qualcosa è andato storto! Probabilmente è un errore momentaneo, riprova per piacere!"). In this case we can also avoid a toast that which would repeat the error information. What you think?
Co-authored-by: Matteo Boschi <[email protected]>
Undermaken
approved these changes
May 26, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Short description:
This pr adds the screen for the "check bonus eligibility" phase.
This screen have a loading state, displaying the
loadingview in table below, and an error view, displaying a message explaining the reasons of the error and two button to cancel o retry.List of changes proposed in this pull request:
ts/features/bonusVacanze/components/GenericLoadingErrorScreen.tsxa generic screen that implements the logic required by this screen, without being tied to the application flow of redux.ts/features/bonusVacanze/screens/CheckBonusEligibilityScreen.tsxa specific screen that tie the generic component with the business logic of the application. At the moment, placeholders are provided instead of concrete actions and reducers.How to test:
In order to test this dummy component, I've just replaced one existing with this one (for example in
MessagesNavigator.tsreplacescreen: MessageHomeScreenwithscreen: CheckBonusEligibilityScreen).