-
Notifications
You must be signed in to change notification settings - Fork 573
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
feat: render new Homeview for artsy people #10851
Conversation
@@ -499,8 +498,7 @@ export const modules = defineModules({ | |||
screenOptions: { | |||
statusBarTranslucent: true, | |||
}, | |||
fullBleed: | |||
ArtsyNativeModule.isBetaOrDev && !unsafe_getFeatureFlag("ARPreferLegacyHomeScreen"), | |||
fullBleed: true, |
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 tested the screens after this change to make sure I didn't break anything
Screen.Recording.2024-09-26.at.17.30.16.mov
|
||
const shouldDisplayNewHomeView = ArtsyNativeModule.isBetaOrDev && !preferLegacyHomeScreen | ||
const shouldDisplayNewHomeView = | ||
(isArtsyEmployee || ArtsyNativeModule.isBetaOrDev) && !preferLegacyHomeScreen |
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.
We want to keep the old logic + the condition of having an artsymail account account
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.
Awesome 👍🏼
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.
Wondering if this check is now out of sync with this one, which was added previously in #10808
eigen/src/app/utils/useHideSplashScreen.ts
Lines 16 to 18 in 1448580
const preferLegacyHomeScreen = useFeatureFlag("ARPreferLegacyHomeScreen") | |
const shouldDisplayNewHomeView = ArtsyNativeModule.isBetaOrDev && !preferLegacyHomeScreen |
Any cause for concern? I.e. will Artsy users on the app store versions miss out on any pre-fetching?
@@ -499,8 +498,7 @@ export const modules = defineModules({ | |||
screenOptions: { | |||
statusBarTranslucent: true, | |||
}, | |||
fullBleed: | |||
ArtsyNativeModule.isBetaOrDev && !unsafe_getFeatureFlag("ARPreferLegacyHomeScreen"), | |||
fullBleed: true, |
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.
Awesome 💯
|
||
const shouldDisplayNewHomeView = ArtsyNativeModule.isBetaOrDev && !preferLegacyHomeScreen | ||
const shouldDisplayNewHomeView = | ||
(isArtsyEmployee || ArtsyNativeModule.isBetaOrDev) && !preferLegacyHomeScreen |
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.
Awesome 👍🏼
This PR resolves ONYX-1330
Description
This PR exposes the new homeview to artsy people.
For easier review, I suggest you hide white spaces
PR Checklist
To the reviewers 👀
Changelog updates
Changelog updates
Cross-platform user-facing changes
iOS user-facing changes
Android user-facing changes
Dev changes
Need help with something? Have a look at our docs, or get in touch with us.