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
[IOCOM-905, IOCOM-906] New DS on Message Router #5755
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5755 +/- ##
==========================================
+ Coverage 48.42% 49.65% +1.22%
==========================================
Files 1488 1608 +120
Lines 31617 31939 +322
Branches 7669 7667 -2
==========================================
+ Hits 15311 15858 +547
+ Misses 16238 16027 -211
+ Partials 68 54 -14
... and 486 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
# Conflicts: # ts/features/messages/screens/__tests__/__snapshots__/MessageRouterScreen.test.tsx.snap
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about hiding the header
on error states? I know that @adelloste worked on this behavior through the react-navigation
navigator. We'll probably discuss this at our next sync meeting so that we get consistent behavior between sections.
Keep in mind that having the |
Also, what if the message loading takes forever? With no back button in the header, the user is stuck on a seemingly non-responsive application. By using it (the back button in the header), the user can cancel the operation and go back to the message list without having to wait for the system timeout (which may not occur at all in case of a very slow connection). |
@shadowsheep1 for a check on the header support in |
@Vangaorth That's why we have two different buttons. We can use the primary one to direct the user to the support flow (even with the precompiled fields, like message ID) and the second one to direct the user to the previous page ("Go back" or "Cancel"). We already manage a similar case in the app (payment flow). We can improve it in a second moment, btw. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Short description
This PR aligns the Message Router screen to the new DS.
Loading
Generic Error
Remote Content Error
Loading
Generic Error
Remote Content Error
List of changes proposed in this pull request
MessageRouterScreen
aligned to the new DSHow to test
Using the io-dev-api-server, check the Message Router screen, in both its loading and failure states.