Skip to content
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

(PC-33485) feat(Modals): resolve modal conflicts #7427

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,59 @@ import { ReactionChoiceModal } from 'features/reactions/components/ReactionChoic
import { ReactionChoiceModalBodyEnum, ReactionFromEnum } from 'features/reactions/enum'
import { OfferImageBasicProps } from 'features/reactions/types'
import { formatToSlashedFrenchDate } from 'libs/dates'
import { useFeatureFlag } from 'libs/firebase/firestore/featureFlags/useFeatureFlag'
import { RemoteStoreFeatureFlags } from 'libs/firebase/firestore/types'
import { useRemoteConfigContext } from 'libs/firebase/remoteConfig/RemoteConfigProvider'
import { reactionModal } from 'libs/modals/modals'
import { useCategoryIdMapping, useSubcategoriesMapping } from 'libs/subcategories'
import { useModal } from 'ui/components/modals/useModal'

export const useIncomingReactionModal = () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

il n'y a pas de tests ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

la complexité cyclomatique est très élevé : 10

const isReactionFeatureActive = useFeatureFlag(RemoteStoreFeatureFlags.WIP_REACTION_FEATURE)
const { reactionCategories } = useRemoteConfigContext()
const { isCookiesListUpToDate, cookiesLastUpdate } = useIsCookiesListUpToDate()
const isCookieConsentChecked = cookiesLastUpdate && isCookiesListUpToDate
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pourquoi les réactions sont couplés aux cookies ?

const { data: bookings } = useBookings()
const subcategoriesMapping = useSubcategoriesMapping()
const mapping = useCategoryIdMapping()

const bookingsWithoutReaction =
bookings?.ended_bookings?.filter((booking) =>
filterBookingsWithoutReaction(booking, subcategoriesMapping, reactionCategories)
) ?? []

const offerImages: OfferImageBasicProps[] = bookingsWithoutReaction.map((current) => {
return {
imageUrl: current.stock.offer.image?.url ?? '',
categoryId: mapping[current.stock.offer.subcategoryId] ?? null,
}
})

const firstBooking = bookingsWithoutReaction[0]
const reactionChoiceModalBodyType =
bookingsWithoutReaction.length === 1
? ReactionChoiceModalBodyEnum.VALIDATION
: ReactionChoiceModalBodyEnum.REDIRECTION
Comment on lines +28 to +44
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

si cette partie était faite dans getModal, on pourrait faire des early return, ce qui permettrait de supprimer plein de ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ça pourrait éviter quelques calculs dans le cas où une autre modal s'affiche avant celle ci


const getModal = () => {
if (!firstBooking || !isCookieConsentChecked || !isReactionFeatureActive) return

const { stock, dateUsed } = firstBooking
const { offer } = stock

return reactionModal({
offer,
dateUsed: dateUsed ? `le ${formatToSlashedFrenchDate(dateUsed)}` : '',
defaultReaction: null,
from: ReactionFromEnum.HOME,
bodyType: reactionChoiceModalBodyType,
offerImages,
})
}

return getModal
}

export const IncomingReactionModalContainer = () => {
const { reactionCategories } = useRemoteConfigContext()
const { isCookiesListUpToDate, cookiesLastUpdate } = useIsCookiesListUpToDate()
Expand Down
49 changes: 27 additions & 22 deletions src/features/home/pages/Home.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,21 @@ import { useAuthContext } from 'features/auth/context/AuthContext'
import { useBookings } from 'features/bookings/api'
import { useHomepageData } from 'features/home/api/useHomepageData'
import { HomeHeader } from 'features/home/components/headers/HomeHeader'
import { IncomingReactionModalContainer } from 'features/home/components/IncomingReactionModalContainer/IncomingReactionModalContainer'
import { useIncomingReactionModal } from 'features/home/components/IncomingReactionModalContainer/IncomingReactionModalContainer'
import { HomeBanner } from 'features/home/components/modules/banners/HomeBanner'
import { PERFORMANCE_HOME_CREATION, PERFORMANCE_HOME_LOADING } from 'features/home/constants'
import { GenericHome } from 'features/home/pages/GenericHome'
import { UseRouteType } from 'features/navigation/RootNavigator/types'
import { AchievementSuccessModal } from 'features/profile/components/Modals/AchievementSuccessModal'
import { useShouldShowAchievementSuccessModal } from 'features/profile/components/Modals/useShouldShowAchievementSuccessModal'
import { OnboardingSubscriptionModal } from 'features/subscription/components/modals/OnboardingSubscriptionModal'
import { useOnboardingSubscriptionModal } from 'features/subscription/helpers/useOnboardingSubscriptionModal'
import { analytics } from 'libs/analytics'
import { useFeatureFlag } from 'libs/firebase/firestore/featureFlags/useFeatureFlag'
import { RemoteStoreFeatureFlags } from 'libs/firebase/firestore/types'
import { useFunctionOnce } from 'libs/hooks'
import { useLocation } from 'libs/location'
import { LocationMode } from 'libs/location/types'
import { createRelativeModals } from 'libs/modals/modal.creator'
import { useModalActions } from 'libs/modals/modal.store'
import { achievementsModal, reactionModal } from 'libs/modals/modals'
import { getAppVersion } from 'libs/packageJson'
import { BatchProfile } from 'libs/react-native-batch'
import { startTransaction } from 'shared/performance/transactions'
Expand All @@ -34,11 +34,15 @@ const Header = () => (
</ListHeaderContainer>
)

let hasShowSessionModal = false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

avoir une variable globale mutable me fait peur sur les effets de bord que ça pourrait avoir


export const Home: FunctionComponent = () => {
const { openModal } = useModalActions()
const startPerfHomeLoadingOnce = useFunctionOnce(() => startTransaction(PERFORMANCE_HOME_LOADING))
const startPerfHomeCreationOnce = useFunctionOnce(() =>
startTransaction(PERFORMANCE_HOME_CREATION)
)

startPerfHomeCreationOnce()
startPerfHomeLoadingOnce()

Expand All @@ -53,26 +57,33 @@ export const Home: FunctionComponent = () => {
showModal: showOnboardingSubscriptionModal,
hideModal: hideOnboardingSubscriptionModal,
} = useModal(false)

useOnboardingSubscriptionModal({
isLoggedIn,
userStatus: user?.status?.statusType,
showOnboardingSubscriptionModal,
})
const { shouldShowAchievementSuccessModal, achievementsToShow } =
useShouldShowAchievementSuccessModal()
const {
visible: visibleAchievementModal,
showModal: showAchievementModal,
hideModal: hideAchievementModal,
} = useModal(false)

const checkAchievementsModal = useShouldShowAchievementSuccessModal()
const checkReactionModal = useIncomingReactionModal()

useEffect(() => {
if (shouldShowAchievementSuccessModal) {
showAchievementModal()
}
}, [shouldShowAchievementSuccessModal, showAchievementModal])
if (hasShowSessionModal) return
hasShowSessionModal = true
const modal = createRelativeModals(
{
creator: reactionModal,
check: checkReactionModal,
},
{
creator: achievementsModal,
check: checkAchievementsModal,
}
)

const isReactionFeatureActive = useFeatureFlag(RemoteStoreFeatureFlags.WIP_REACTION_FEATURE)
if (!modal) return
openModal(modal)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

si on arrive depuis un deep link sur autre page que la home, on ne verra pas les modales parce qu'on n'est pas passé par la home

est-ce le comportement voulu ?

Comment on lines +71 to +85
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

où sont les tests ?

}, [openModal, checkReactionModal, checkAchievementsModal])

useEffect(() => {
if (id) {
Expand Down Expand Up @@ -139,12 +150,6 @@ export const Home: FunctionComponent = () => {
visible={onboardingSubscriptionModalVisible}
dismissModal={hideOnboardingSubscriptionModal}
/>
{isReactionFeatureActive ? <IncomingReactionModalContainer /> : null}
<AchievementSuccessModal
names={achievementsToShow}
visible={visibleAchievementModal}
hideModal={hideAchievementModal}
/>
</React.Fragment>
)
}
Expand Down
3 changes: 3 additions & 0 deletions src/features/navigation/RootNavigator/RootNavigator.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import { withWebWrapper } from 'features/navigation/RootNavigator/withWebWrapper
import { TabNavigationStateProvider } from 'features/navigation/TabBar/TabNavigationStateContext'
import { VenueMapFiltersStackNavigator } from 'features/navigation/VenueMapFiltersStackNavigator/VenueMapFiltersStackNavigator'
import { AccessibilityRole } from 'libs/accessibilityRole/accessibilityRole'
import { ModalRenderer } from 'libs/modals/modal.renderer'
import { modalFactory } from 'libs/modals/modals'
import { useSplashScreenContext } from 'libs/splashscreen'
import { storage } from 'libs/storage'
import { IconFactoryProvider } from 'ui/components/icons/IconFactoryProvider'
Expand Down Expand Up @@ -108,6 +110,7 @@ export const RootNavigator: React.ComponentType = () => {
{/* The components below are those for which we do not want
their rendering to happen while the splash is displayed. */}
{isSplashScreenHidden ? <PrivacyPolicy /> : null}
<ModalRenderer modalFactory={modalFactory} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PrivacyPolicy s'affiche uniquement si le splash screen n'est plus affiché

est-ce qu'il n'y aurait pas un risque d'afficher les modales au moment du splash screen ?

</TabNavigationStateProvider>
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import styled from 'styled-components/native'
import { AchievementEnum } from 'api/gen'
import { analytics } from 'libs/analytics'
import LottieView from 'libs/lottie'
import { achievementsModal, modalFactory } from 'libs/modals/modals'
import { ButtonPrimary } from 'ui/components/buttons/ButtonPrimary'
import { ButtonTertiaryBlack } from 'ui/components/buttons/ButtonTertiaryBlack'
import { AppInformationModal } from 'ui/components/modals/AppInformationModal'
Expand All @@ -21,6 +22,10 @@ interface Props {
names: AchievementEnum[]
}

modalFactory.add(achievementsModal, ({ params: { names }, close }) => {
return <AchievementSuccessModal visible hideModal={close} names={names} />
})

export const AchievementSuccessModal = ({ visible, hideModal, names }: Props) => {
const logoRef = useRef<LottieView>(null)

Expand All @@ -33,7 +38,7 @@ export const AchievementSuccessModal = ({ visible, hideModal, names }: Props) =>
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [visible])

if (!visible || names.length <= 0) return null
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pourquoi on n'a plus besoin de cette condition ?

if (!visible) return null

const severalAchievementsUnlocked = names.length >= 2

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,33 +3,38 @@ import { useAuthContext } from 'features/auth/context/AuthContext'
import { useFeatureFlag } from 'libs/firebase/firestore/featureFlags/useFeatureFlag'
import { RemoteStoreFeatureFlags } from 'libs/firebase/firestore/types'
import { useRemoteConfigContext } from 'libs/firebase/remoteConfig/RemoteConfigProvider'
import { achievementsModal } from 'libs/modals/modals'

export const useShouldShowAchievementSuccessModal = () => {
const areAchievementsEnabled = useFeatureFlag(RemoteStoreFeatureFlags.ENABLE_ACHIEVEMENTS)
const { displayAchievements } = useRemoteConfigContext()
const { user } = useAuthContext()

let shouldShowAchievementSuccessModal = false
let achievementsToShow: AchievementEnum[] = []
if (
!areAchievementsEnabled ||
!displayAchievements ||
!user?.achievements ||
user?.achievements.length === 0
)
return { shouldShowAchievementSuccessModal, achievementsToShow }

const isThereAtLeastOneUnseenAchievement = user?.achievements.some(
(achievement) => !achievement.seenDate
)
if (isThereAtLeastOneUnseenAchievement) shouldShowAchievementSuccessModal = true
else return { shouldShowAchievementSuccessModal, achievementsToShow }

const unseenAchievements = user?.achievements
.filter((achievement) => !achievement.seenDate)
.map((achievement) => achievement.name)

if (unseenAchievements && unseenAchievements?.length > 0) achievementsToShow = unseenAchievements

return { shouldShowAchievementSuccessModal, achievementsToShow }
const getModal = () => {
let achievementsToShow: AchievementEnum[] = []
if (
!areAchievementsEnabled ||
!displayAchievements ||
!user?.achievements ||
user?.achievements.length === 0
)
return

const isThereAtLeastOneUnseenAchievement = user?.achievements.some(
(achievement) => !achievement.seenDate
)

if (!isThereAtLeastOneUnseenAchievement) return

const unseenAchievements = user?.achievements
.filter((achievement) => !achievement.seenDate)
.map((achievement) => achievement.name)

if (unseenAchievements && unseenAchievements?.length > 0)
achievementsToShow = unseenAchievements

return achievementsModal({ names: achievementsToShow })
}

return getModal
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { ReactionChoiceModalBodyWithValidation } from 'features/reactions/compon
import { ReactionChoiceModalBodyEnum, ReactionFromEnum } from 'features/reactions/enum'
import { OfferImageBasicProps } from 'features/reactions/types'
import { analytics } from 'libs/analytics'
import { modalFactory, reactionModal } from 'libs/modals/modals'
import { ButtonPrimary } from 'ui/components/buttons/ButtonPrimary'
import { ButtonTertiaryBlack } from 'ui/components/buttons/ButtonTertiaryBlack'
import { AppModal } from 'ui/components/modals/AppModal'
Expand All @@ -38,6 +39,10 @@ type Props = {
offerImages?: OfferImageBasicProps[]
}

modalFactory.add(reactionModal, ({ params, close }) => {
return <ReactionChoiceModal visible {...params} closeModal={close} />
})

export const ReactionChoiceModal: FunctionComponent<Props> = ({
offer,
dateUsed,
Expand Down
22 changes: 22 additions & 0 deletions src/libs/modals/__tests__/close-modal.native.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { modalStore } from '../modal.store'

describe('Feature: Close modal', () => {
test('Modal is closed', () => {
const modal = { key: 'modal-key', params: { text: 'Hello world!' } }
modalStore.setState({ modalOpened: modal })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cet état ne devrait pas etre possible : d'après le typage actuel, il y a toujours une queue meme si elle est vide

si on passe par les actions exposées par le module, ça devrait etre ok

Suggested change
modalStore.setState({ modalOpened: modal })
modalStore.getState().actions.openModal(modal)


modalStore.getState().actions.closeModal()

expect(modalStore.getState().modalOpened).toBeUndefined()
})

test('Modal is opened from queue if there is one', () => {
const modal = { key: 'modal-key', params: { text: 'Hello world!' } }
const queuedModal = { key: 'queued-modal-key', params: { text: 'Hello world!' } }
modalStore.setState({ modalOpened: modal, queue: [queuedModal] })

modalStore.getState().actions.closeModal()

expect(modalStore.getState().modalOpened).toEqual(queuedModal)
})
})
78 changes: 78 additions & 0 deletions src/libs/modals/__tests__/modal-relative.native.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import { createModal, createRelativeModals } from '../modal.creator'

describe('Feature: Modal relative', () => {
test('Modal is returned', () => {
const firstModal = createModal<{ title: string }>('first')
const relativeModal = createRelativeModals({
creator: firstModal,
check() {
return firstModal({ title: 'hello world' })
},
})

expect(relativeModal).toEqual(firstModal({ title: 'hello world' }))
})

test('Second modal is returned when first modal has not to be open', () => {
const firstModal = createModal<{ title: string }>('first')
const secondModal = createModal<{ name: string }>('second')
const relativeModal = createRelativeModals(
{
creator: firstModal,
check() {
return undefined
},
},
{
creator: secondModal,
check() {
return secondModal({ name: 'hello world' })
},
}
)

expect(relativeModal).toEqual(secondModal({ name: 'hello world' }))
})

test('Return the first modal to be open', () => {
const firstModal = createModal<{ title: string }>('first')
const secondModal = createModal<{ name: string }>('second')
const relativeModal = createRelativeModals(
{
creator: firstModal,
check() {
return firstModal({ title: 'hello world' })
},
},
{
creator: secondModal,
check() {
return secondModal({ name: 'hello world' })
},
}
)

expect(relativeModal).toEqual(firstModal({ title: 'hello world' }))
})

test('Return undefined when no modal has to be open', () => {
const firstModal = createModal<{ title: string }>('first')
const secondModal = createModal<{ name: string }>('second')
const relativeModal = createRelativeModals(
{
creator: firstModal,
check() {
return undefined
},
},
{
creator: secondModal,
check() {
return undefined
},
}
)

expect(relativeModal).toBeUndefined()
})
})
Loading
Loading