-
Notifications
You must be signed in to change notification settings - Fork 9
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
Improve caldav calendar settings flow #786
Improve caldav calendar settings flow #786
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.
Looks good! (Only commenting, as I haven't tested it)
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.
Looks good, I almost wonder if we should merge the single calendar connection with the modal.
Speaking of the modal, does this change affect the modal's caldav autodiscovery connection functionality? (i.e. do these errors show up properly on there too?)
You mean the FTUE modal? The Authentication Error should show up there too, the rest of the change shouldn't effect it. I only took care of the calendar settings section with this. Should we create another ticket for FTUE? |
At least for the investigation post this PR, and any work that may come from that. Thanks! |
Done (see #788). Thanks for the reviews! |
Description of the Change
This change improves the setup of new CalDAV connections.
Benefits
Better feedback for users on CalDAV setup in settings.
Applicable Issues
Closes #777