-
Notifications
You must be signed in to change notification settings - Fork 21
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
IGAPP-1130: Native Event Export #949
Conversation
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.
Tested on android and ios. Works fine. :)
I think in the CalendarChoice should have a back button. especially for ios (i couldn't test the modal there because i don't have multiple calendars on simulator)
If the calendar permission is denied once it should be possible to re-request the permission somehow imo. Maybe show next to the error a button to re-request it.
Co-authored-by: Andreas Fischer <[email protected]>
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.
One small thing. The "schließen" Button is not very prominent. Why not using arrow back in the left corner with an modal header like in the feedback modal to have the same look and feel
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.
Taseted on Android and simulated ios. Nice result 👍🏼
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.
Three things I realized:
- The
Add to Calendar
button flickers quickly bc its rendered before the page content is rendered. UsingAfterContent
instead ofpageFooter
should fix that, you just have to apply additional styling to avoid the button spanning the whole width. - The text of the system status bar on android is white and therefore not readable, see screenshot.
- The header of the modal is not stuck to the top, see screenshot.

I am not 100% convinced that we should use a modal for this (we could also use a new route like for ChangeLanguageModal) but it seems to be the easiest solution for now. However, it should correspond to the layout/design of our other screens, mainly the header.
I tested this already and afaics it works great, nicely done!
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.
I like how smooth this all works together! Now I find integreat events all the time in my calendars though after testing 😎
Sadly the text in the status bar on android is white and therefore not readable. Not sure why this happens.
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.
Seems like that the status bar prop barStyle
just gets ignored if you render a modal (at least on my device). I have a custom rom installed so in theory that could also be the problem. I checked both light and dark mode and same issue either way.
Maybe someone could test this with a real android device as well?
Another note:
In landscape mode it doesn't look very good 🙈 The react native docs also say that as default on ios only portrait works.
Co-authored-by: Steffen Kleinle <[email protected]>
Co-authored-by: Steffen Kleinle <[email protected]>
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.
Not tested again. Would be cool if someone else could test on another real android device. If not reproducible, feel free to merge.
We figured out that it is an issue in some Android devices that the StatusBar gets a light content when a Modal is opened (facebook/react-native#34350). There might be a solution in RN 0.71, will do that update and then check again. |
This pull request belongs to the issue https://issues.tuerantuer.org/browse/IGAPP-1130 .
I used an external library in the end, and opened a snackbar with a button to the calendar on success.
It doesn't handle recurring events yet because we haven't made a decision about how to export those yet.