-
Notifications
You must be signed in to change notification settings - Fork 59
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Better external auth integration, restore + site button, tests! #223
Conversation
Authentication now uses ephemeral session, plus captures external auth properly on sites that use it. Also adds a button to add a single site much like we did before adding Discover to the app (requested by users on meta).
Similar to what Safari does when it runs into a crash, should help with odd crashes on iPad. Reported in https://meta.discourse.org/t/327190
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.
At last, we can give iOS only 1 icon file and have it handle all sizes on its own!
@@ -561,8 +547,9 @@ class Discourse extends React.Component { | |||
tabBarBackground: () => this._blurView(theme.name), | |||
}}> | |||
<Tab.Screen | |||
name={i18n.t('home')} | |||
name="Home" |
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 know this was intentional but it's ok to lose i18n?
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.
Oh wow I'm an idiot. I see it now.
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.
Yeah, this was a pretty stupid regression on my part. It caused some ugly routing errors for users on non-English locales. (That's why there are tests for this now.)
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.
All the logic looks good to me! I'm not pulling it down and testing it -- you said it has been running for a bit for a while on several devices.
Thanks @markvanlan! |
Various fixes:
Plus, some basic e2e tests have been added!