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.

Sign up for GitHub

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

Jump to bottom

feat: Inline price control create alert #8835

Merged
merged 9 commits into from
Jun 20, 2023
Merged

feat: Inline price control create alert #8835

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
2e75feb
inline price control create alert
rajsam003 Jun 8, 2023
37ec7f0
update
rajsam003 Jun 8, 2023
9cf5c7e
revert podlock file
rajsam003 Jun 8, 2023
dfc9669
new screen
rajsam003 Jun 9, 2023
56592d6
with Edit Alert flow
rajsam003 Jun 12, 2023
0ddc6cf
price update
rajsam003 Jun 12, 2023
dff9b55
remove section
rajsam003 Jun 13, 2023
add643e
updated with PR
rajsam003 Jun 16, 2023
4a41fef
AlertPriceRangeScreen to AlertPriceRange
rajsam003 Jun 16, 2023
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
32 changes: 31 additions & 1 deletion src/app/Scenes/SavedSearchAlert/Components/Form.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,19 @@
import { CloseIcon, Spacer, Flex, Box, Text, Button } from "@artsy/palette-mobile"
import {
CloseIcon,
Spacer,
Flex,
Box,
Text,
Button,
ArrowRightIcon,
Touchable,
} from "@artsy/palette-mobile"
import { NavigationProp, useNavigation } from "@react-navigation/native"
import { SearchCriteria } from "app/Components/ArtworkFilter/SavedSearch/types"
import { Input, InputTitle } from "app/Components/Input"
import { Pill } from "app/Components/Pill"
import {
CreateSavedSearchAlertNavigationStack,
SavedSearchAlertFormValues,
SavedSearchPill,
} from "app/Scenes/SavedSearchAlert/SavedSearchAlertModel"
Expand Down Expand Up @@ -41,6 +52,8 @@ export const Form: React.FC<FormProps> = (props) => {
} = props
const { isSubmitting, values, errors, dirty, handleBlur, handleChange } =
useFormikContext<SavedSearchAlertFormValues>()
const navigation =
useNavigation<NavigationProp<CreateSavedSearchAlertNavigationStack, "CreateSavedSearchAlert">>()
const entity = SavedSearchStore.useStoreState((state) => state.entity)
const isEditMode = !!savedSearchAlertId
let isSaveAlertButtonDisabled = false
Expand Down Expand Up @@ -125,6 +138,23 @@ export const Form: React.FC<FormProps> = (props) => {
))}
</Flex>
</Box>
<Spacer y={2} />
<Touchable
rajsam003 marked this conversation as resolved.
Show resolved Hide resolved
accessibilityLabel="Set price range"
accessibilityRole="button"
underlayColor="black5"
Copy link
Member

Choose a reason for hiding this comment

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

I think you can leave this out, no longer needed unless you want a custom color (you can see that removing it has the same behavior while pressing it)

Suggested change
underlayColor="black5"

onPress={() => navigation.navigate("AlertPriceRangeScreen")}
>
<Flex flexDirection="row" alignItems="center" py={1}>
rajsam003 marked this conversation as resolved.
Show resolved Hide resolved
<Flex flex={1}>
<Text variant="sm-display">Set price range you are interested in</Text>
</Flex>
<Flex alignSelf="center" mt={0.5}>
<ArrowRightIcon />
</Flex>
</Flex>
</Touchable>
<Spacer y={4} />
<SavedSearchAlertSwitch
label="Mobile Alerts"
onChange={onTogglePushNotification}
Expand Down
63 changes: 40 additions & 23 deletions src/app/Scenes/SavedSearchAlert/CreateSavedSearchAlert.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,41 +1,58 @@
import { Box } from "@artsy/palette-mobile"
import { NavigationContainer } from "@react-navigation/native"
import { createStackNavigator, TransitionPresets } from "@react-navigation/stack"
import { SearchCriteriaAttributes } from "app/Components/ArtworkFilter/SavedSearch/types"
import { FancyModal } from "app/Components/FancyModal/FancyModal"
import {
savedSearchModel,
SavedSearchStoreProvider,
} from "app/Scenes/SavedSearchAlert/SavedSearchStore"
import { CreateSavedSearchAlertContentQueryRenderer } from "app/Scenes/SavedSearchAlert/containers/CreateSavedSearchContentContainer"
import { AlertPriceRangeScreen } from "app/Scenes/SavedSearchAlert/screens/AlertPriceRangeScreen"
import {
CreateSavedSearchAlertNavigationStack,
CreateSavedSearchAlertProps,
} from "./SavedSearchAlertModel"
import { CreateSavedSearchAlertScreen } from "./screens/CreateSavedSearchAlertScreen"
import { EmailPreferencesScreen } from "./screens/EmailPreferencesScreen"

const Stack = createStackNavigator<CreateSavedSearchAlertNavigationStack>()

export const CreateSavedSearchAlert: React.FC<CreateSavedSearchAlertProps> = (props) => {
const { visible, params } = props
const { attributes, aggregations, entity } = params

return (
<NavigationContainer independent>
<FancyModal visible={visible} fullScreen>
<Box flex={1}>
<Stack.Navigator
// force it to not use react-native-screens, which is broken inside a react-native Modal for some reason
detachInactiveScreens={false}
screenOptions={{
...TransitionPresets.SlideFromRightIOS,
headerShown: false,
cardStyle: { backgroundColor: "white" },
}}
>
<Stack.Screen
name="CreateSavedSearchAlert"
component={CreateSavedSearchAlertScreen}
initialParams={params}
/>
<Stack.Screen name="EmailPreferences" component={EmailPreferencesScreen} />
</Stack.Navigator>
</Box>
</FancyModal>
</NavigationContainer>
<SavedSearchStoreProvider
runtimeModel={{
...savedSearchModel,
attributes: attributes as SearchCriteriaAttributes,
aggregations,
entity,
}}
>
gkartalis marked this conversation as resolved.
Show resolved Hide resolved
<NavigationContainer independent>
<FancyModal visible={visible} fullScreen>
<Box flex={1}>
<Stack.Navigator
// force it to not use react-native-screens, which is broken inside a react-native Modal for some reason
detachInactiveScreens={false}
Comment on lines +37 to +38
Copy link
Member

Choose a reason for hiding this comment

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

tell me more 👀 we might need to be more proactive about this in other surfaces maybe, how did you find out?

Copy link
Member

Choose a reason for hiding this comment

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

@gkartalis maybe you and @rajsam003 can find some time to pair on this? Looks like this argument, and the same comment, exists already in a few different screens.

Copy link
Member

Choose a reason for hiding this comment

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

good point, yes def @rajsam003 feel free to drop some time next week on my cal

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As I said I referred other screens as well ( for eg. here, here. It seemed like its generally the case in lot of places. So I find it safer to use this prop.

Copy link
Member

Choose a reason for hiding this comment

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

We looked into it with @rajsam003, turns out that there is an issue with react native modals as the comment suggests, not sure how we can resolve it and remove it but we confirmed that removing this line it breaks the flow (the set price screen modal cannot open).

so lets leave it as it is (MOPLAT will further investigate when migrating to react-navigation.

screenOptions={{
...TransitionPresets.SlideFromRightIOS,
headerShown: false,
cardStyle: { backgroundColor: "white" },
}}
>
<Stack.Screen
name="CreateSavedSearchAlert"
component={CreateSavedSearchAlertContentQueryRenderer}
initialParams={params}
/>
<Stack.Screen name="EmailPreferences" component={EmailPreferencesScreen} />
<Stack.Screen name="AlertPriceRangeScreen" component={AlertPriceRangeScreen} />
</Stack.Navigator>
</Box>
</FancyModal>
</NavigationContainer>
</SavedSearchStoreProvider>
)
}
69 changes: 46 additions & 23 deletions src/app/Scenes/SavedSearchAlert/EditSavedSearchAlert.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { OwnerType } from "@artsy/cohesion"
import { NavigationContainer } from "@react-navigation/native"
import { TransitionPresets, createStackNavigator } from "@react-navigation/stack"
import { EditSavedSearchAlertQuery } from "__generated__/EditSavedSearchAlertQuery.graphql"
import { EditSavedSearchAlert_artists$data } from "__generated__/EditSavedSearchAlert_artists.graphql"
import { EditSavedSearchAlert_artworksConnection$data } from "__generated__/EditSavedSearchAlert_artworksConnection.graphql"
Expand All @@ -10,7 +12,13 @@ import {
SavedSearchEntityArtist,
SearchCriteriaAttributes,
} from "app/Components/ArtworkFilter/SavedSearch/types"
import { PageWithSimpleHeader } from "app/Components/PageWithSimpleHeader"
import { EditSavedSearchAlertContent } from "app/Scenes/SavedSearchAlert/EditSavedSearchAlertContent"
import {
EditSavedSearchAlertNavigationStack,
EditSavedSearchAlertParams,
} from "app/Scenes/SavedSearchAlert/SavedSearchAlertModel"
import { AlertPriceRangeScreen } from "app/Scenes/SavedSearchAlert/screens/AlertPriceRangeScreen"
import { EmailPreferencesScreen } from "app/Scenes/SavedSearchAlert/screens/EmailPreferencesScreen"
import { goBack, GoBackProps, navigationEvents } from "app/system/navigation/navigate"
import { getRelayEnvironment } from "app/system/relay/defaultEnvironment"
import { ArtsyKeyboardAvoidingView } from "app/utils/ArtsyKeyboardAvoidingView"
Expand All @@ -20,7 +28,6 @@ import React, { useCallback, useEffect } from "react"
import { createRefetchContainer, graphql, QueryRenderer, RelayRefetchProp } from "react-relay"
import { EditSavedSearchFormPlaceholder } from "./Components/EditSavedSearchAlertPlaceholder"
import { SavedSearchAlertQueryRenderer } from "./SavedSearchAlert"
import { SavedSearchAlertForm } from "./SavedSearchAlertForm"
import { SavedSearchStoreProvider, savedSearchModel } from "./SavedSearchStore"

interface EditSavedSearchAlertBaseProps {
Expand All @@ -36,6 +43,8 @@ interface EditSavedSearchAlertProps {
relay: RelayRefetchProp
}

const Stack = createStackNavigator<EditSavedSearchAlertNavigationStack>()

export const EditSavedSearchAlert: React.FC<EditSavedSearchAlertProps> = (props) => {
const { me, viewer, artists, artworksConnection, savedSearchAlertId, relay } = props
const aggregations = (artworksConnection.aggregations ?? []) as Aggregations
Expand Down Expand Up @@ -86,6 +95,14 @@ export const EditSavedSearchAlert: React.FC<EditSavedSearchAlertProps> = (props)
}
}, [])

const params: EditSavedSearchAlertParams = {
userAlertSettings,
savedSearchAlertId,
userAllowsEmails,
onComplete,
onDeleteComplete: onComplete,
}

return (
<ProvideScreenTracking
info={{
Expand All @@ -95,28 +112,34 @@ export const EditSavedSearchAlert: React.FC<EditSavedSearchAlertProps> = (props)
}}
>
<ArtsyKeyboardAvoidingView>
<PageWithSimpleHeader title="Edit your Alert">
<SavedSearchStoreProvider
runtimeModel={{
...savedSearchModel,
attributes: attributes as SearchCriteriaAttributes,
aggregations,
entity,
}}
>
<SavedSearchAlertForm
initialValues={{
name: userAlertSettings?.name ?? "",
email: userAlertSettings?.email ?? false,
push: userAlertSettings?.push ?? false,
<SavedSearchStoreProvider
runtimeModel={{
...savedSearchModel,
attributes: attributes as SearchCriteriaAttributes,
aggregations,
entity,
}}
>
<NavigationContainer independent>
<Stack.Navigator
// force it to not use react-native-screens, which is broken inside a react-native Modal for some reason
detachInactiveScreens={false}
screenOptions={{
...TransitionPresets.SlideFromRightIOS,
headerShown: false,
cardStyle: { backgroundColor: "white" },
}}
savedSearchAlertId={savedSearchAlertId}
userAllowsEmails={userAllowsEmails}
onComplete={onComplete}
onDeleteComplete={onComplete}
/>
</SavedSearchStoreProvider>
</PageWithSimpleHeader>
>
<Stack.Screen
name="EditSavedSearchAlertContent"
component={EditSavedSearchAlertContent}
initialParams={params}
/>
<Stack.Screen name="EmailPreferences" component={EmailPreferencesScreen} />
<Stack.Screen name="AlertPriceRangeScreen" component={AlertPriceRangeScreen} />
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: Where is name used here? We should probably be consistent with including or omitting the Screen suffix in the string value.

</Stack.Navigator>
</NavigationContainer>
</SavedSearchStoreProvider>
</ArtsyKeyboardAvoidingView>
</ProvideScreenTracking>
)
Expand Down
27 changes: 27 additions & 0 deletions src/app/Scenes/SavedSearchAlert/EditSavedSearchAlertContent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { RouteProp, useRoute } from "@react-navigation/native"
import { PageWithSimpleHeader } from "app/Components/PageWithSimpleHeader"
import { SavedSearchAlertForm } from "app/Scenes/SavedSearchAlert/SavedSearchAlertForm"
import { EditSavedSearchAlertNavigationStack } from "app/Scenes/SavedSearchAlert/SavedSearchAlertModel"

export const EditSavedSearchAlertContent = () => {
const route =
useRoute<RouteProp<EditSavedSearchAlertNavigationStack, "EditSavedSearchAlertContent">>()
const { userAlertSettings, savedSearchAlertId, userAllowsEmails, onComplete, onDeleteComplete } =
route.params

return (
<PageWithSimpleHeader title="Edit your Alert">
<SavedSearchAlertForm
initialValues={{
name: userAlertSettings?.name ?? "",
email: userAlertSettings?.email ?? false,
MounirDhahri marked this conversation as resolved.
Show resolved Hide resolved
push: userAlertSettings?.push ?? false,
}}
savedSearchAlertId={savedSearchAlertId}
userAllowsEmails={userAllowsEmails}
onComplete={onComplete}
onDeleteComplete={onDeleteComplete}
/>
</PageWithSimpleHeader>
)
}
19 changes: 19 additions & 0 deletions src/app/Scenes/SavedSearchAlert/SavedSearchAlertModel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,25 @@ export interface CreateSavedSearchAlertProps {

export type CreateSavedSearchAlertNavigationStack = {
CreateSavedSearchAlert: CreateSavedSearchAlertParams
AlertPriceRangeScreen: undefined
EmailPreferences: undefined
}

export interface EditSavedSearchAlertParams {
userAlertSettings?: {
name: string | null
email: boolean
push: boolean
}
savedSearchAlertId?: string
userAllowsEmails: boolean
onComplete?: (result: SavedSearchAlertMutationResult) => void
onDeleteComplete?: () => void
}

export type EditSavedSearchAlertNavigationStack = {
EditSavedSearchAlertContent: EditSavedSearchAlertParams
AlertPriceRangeScreen: undefined
EmailPreferences: undefined
}

Expand Down
17 changes: 17 additions & 0 deletions src/app/Scenes/SavedSearchAlert/SavedSearchStore.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@ interface SavedSearchModel {
entity: SavedSearchEntity
dirty: boolean

setValueToAttributesByKeyAction: Action<
this,
{
key: SearchCriteria
value: string | number | boolean
}
>
removeValueFromAttributesByKeyAction: Action<
this,
{
Expand All @@ -36,6 +43,16 @@ export const savedSearchModel: SavedSearchModel = {
},
},

setValueToAttributesByKeyAction: action((state, payload) => {
// set form dirty on price update
if (payload.key === "priceRange" && state.attributes[payload.key] !== payload.value) {
state.dirty = true
}

//@ts-ignore
state.attributes[payload.key] = payload.value
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I find it hard to fix this type error. Any help is appreciated.

}),

removeValueFromAttributesByKeyAction: action((state, payload) => {
const prevValue = state.attributes[payload.key]

Expand Down
25 changes: 10 additions & 15 deletions src/app/Scenes/SavedSearchAlert/containers/CreateSavedSearchContentContainer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
import { Box } from "@artsy/palette-mobile"
import { useFocusEffect } from "@react-navigation/core"
import { StackNavigationProp } from "@react-navigation/stack"
import { NavigationProp, RouteProp, useNavigation, useRoute } from "@react-navigation/native"
import { captureMessage } from "@sentry/react-native"
import { CreateSavedSearchContentContainerQuery } from "__generated__/CreateSavedSearchContentContainerQuery.graphql"
import { CreateSavedSearchContentContainer_viewer$data } from "__generated__/CreateSavedSearchContentContainer_viewer.graphql"
import { FancyModalHeader } from "app/Components/FancyModal/FancyModalHeader"
import { SavedSearchAlertForm } from "app/Scenes/SavedSearchAlert/SavedSearchAlertForm"
import {
CreateSavedSearchAlertNavigationStack,
SavedSearchAlertMutationResult,
} from "app/Scenes/SavedSearchAlert/SavedSearchAlertModel"
import { CreateSavedSearchAlertNavigationStack } from "app/Scenes/SavedSearchAlert/SavedSearchAlertModel"
import { getRelayEnvironment } from "app/system/relay/defaultEnvironment"
import {
getNotificationPermissionsStatus,
Expand All @@ -19,21 +16,19 @@ import useAppState from "app/utils/useAppState"
import React, { useCallback, useEffect, useRef, useState } from "react"
import { createRefetchContainer, graphql, QueryRenderer, RelayRefetchProp } from "react-relay"

interface CreateSavedSearchAlertContentQueryRendererProps {
navigation: StackNavigationProp<CreateSavedSearchAlertNavigationStack, "CreateSavedSearchAlert">
onClosePress: () => void
onComplete: (response: SavedSearchAlertMutationResult) => void
}

interface CreateSavedSearchAlertContentProps
extends CreateSavedSearchAlertContentQueryRendererProps {
interface CreateSavedSearchAlertContentProps {
relay: RelayRefetchProp
viewer?: CreateSavedSearchContentContainer_viewer$data | null
loading: boolean
}

const CreateSavedSearchAlertContent: React.FC<CreateSavedSearchAlertContentProps> = (props) => {
const { viewer, loading, relay, navigation, onClosePress, ...other } = props
const navigation =
useNavigation<NavigationProp<CreateSavedSearchAlertNavigationStack, "CreateSavedSearchAlert">>()
const route =
useRoute<RouteProp<CreateSavedSearchAlertNavigationStack, "CreateSavedSearchAlert">>()
const { viewer, loading, relay } = props
const { onClosePress, ...other } = route.params
const isPreviouslyFocused = useRef(false)
const [refetching, setRefetching] = useState(false)
const [enablePushNotifications, setEnablePushNotifications] = useState(true)
Expand Down Expand Up @@ -125,7 +120,7 @@ const CreateSavedSearchContentContainer = createRefetchContainer(
)

export const CreateSavedSearchAlertContentQueryRenderer: React.FC<
CreateSavedSearchAlertContentQueryRendererProps
CreateSavedSearchAlertContentProps
> = (props) => {
return (
<QueryRenderer<CreateSavedSearchContentContainerQuery>
Expand Down
Loading