-
Notifications
You must be signed in to change notification settings - Fork 279
(wrong branch - feta) #5901
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
(wrong branch - feta) #5901
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| diff --git a/node_modules/react-native-store-review/RNStoreReview.podspec b/node_modules/react-native-store-review/RNStoreReview.podspec | ||
| index 1234567..abcdefg 100644 | ||
| --- a/node_modules/react-native-store-review/RNStoreReview.podspec | ||
| +++ b/node_modules/react-native-store-review/RNStoreReview.podspec | ||
| @@ -14,6 +14,10 @@ Pod::Spec.new do |s| | ||
| s.preserve_paths = "**/*.js" | ||
| s.ios.framework = 'StoreKit' | ||
| s.requires_arc = true | ||
| + s.pod_target_xcconfig = { | ||
| + 'DEFINES_MODULE' => 'YES', | ||
| + 'SWIFT_VERSION' => '5.0' | ||
| + } | ||
|
|
||
| s.dependency "React-Core" | ||
|
|
||
| diff --git a/node_modules/react-native-store-review/ios/RNStoreReview.mm b/node_modules/react-native-store-review/ios/RNStoreReview.mm | ||
| index 2345678..bcdefgh 100644 | ||
| --- a/node_modules/react-native-store-review/ios/RNStoreReview.mm | ||
| +++ b/node_modules/react-native-store-review/ios/RNStoreReview.mm | ||
| @@ -1,6 +1,6 @@ | ||
| #import "RNStoreReview.h" | ||
|
|
||
| -#import "RNStoreReview-Swift.h" | ||
| +#import <RNStoreReview/RNStoreReview-Swift.h> | ||
|
|
||
| @implementation RNStoreReview | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,7 +35,7 @@ import { checkAndRequestPermission } from '../services/PermissionsManager' | |
| import { cacheStyles, type Theme, useTheme } from '../services/ThemeContext' | ||
| import { EdgeText, Paragraph } from '../themed/EdgeText' | ||
| import { ModalFooter } from '../themed/ModalParts' | ||
| import { SceneHeader } from '../themed/SceneHeader' | ||
| import { SceneHeaderUi4 } from '../themed/SceneHeaderUi4' | ||
| import { EdgeModal } from './EdgeModal' | ||
|
|
||
| interface Props { | ||
|
|
@@ -52,7 +52,7 @@ interface Props { | |
| textModalTitle?: string | ||
| } | ||
|
|
||
| export const ScanModal = (props: Props) => { | ||
| export const ScanModal: React.FC<Props> = props => { | ||
| const { | ||
| bridge, | ||
| textModalAutoFocus, | ||
|
|
@@ -75,38 +75,43 @@ export const ScanModal = (props: Props) => { | |
| handleBarCodeRead(codes) | ||
| } | ||
| }) | ||
| const cameraPermission = useSelector(state => state.permissions.camera) | ||
| const reduxCameraPermission = useSelector(state => state.permissions.camera) | ||
| const [cameraPermission, setCameraPermission] = React.useState( | ||
| reduxCameraPermission | ||
| ) | ||
| const [torchEnabled, setTorchEnabled] = React.useState(false) | ||
| const [scanEnabled, setScanEnabled] = React.useState(false) | ||
|
|
||
| const handleFlash = () => { | ||
| const handleFlash = (): void => { | ||
| triggerHaptic('impactLight') | ||
| setTorchEnabled(!torchEnabled) | ||
| } | ||
|
|
||
| // Mount effects | ||
| React.useEffect(() => { | ||
| setScanEnabled(true) | ||
| checkAndRequestPermission('camera').catch(err => { | ||
| showError(err) | ||
| }) | ||
| checkAndRequestPermission('camera') | ||
| .then(status => { | ||
| setCameraPermission(status) | ||
| }) | ||
| .catch(showError) | ||
| return () => { | ||
| setScanEnabled(false) | ||
| } | ||
| }, []) | ||
|
|
||
| const handleBarCodeRead = (codes: Code[]) => { | ||
| const handleBarCodeRead = (codes: Code[]): void => { | ||
| setScanEnabled(false) | ||
| triggerHaptic('impactLight') | ||
| bridge.resolve(codes[0].value) | ||
| } | ||
|
|
||
| const handleSettings = async () => { | ||
| const handleSettings = async (): Promise<void> => { | ||
| triggerHaptic('impactLight') | ||
| await Linking.openSettings() | ||
| } | ||
|
|
||
| const handleTextInput = async () => { | ||
| const handleTextInput = async (): Promise<void> => { | ||
| triggerHaptic('impactLight') | ||
| const uri = await Airship.show<string | undefined>(bridge => ( | ||
| <TextInputModal | ||
|
|
@@ -124,16 +129,16 @@ export const ScanModal = (props: Props) => { | |
| } | ||
| } | ||
|
|
||
| const handleAlbum = () => { | ||
| const handleAlbum = (): void => { | ||
| triggerHaptic('impactLight') | ||
| launchImageLibrary( | ||
| { | ||
| mediaType: 'photo' | ||
| }, | ||
| result => { | ||
| if (result.didCancel) return | ||
| if (result.didCancel === true) return | ||
|
|
||
| if (result.errorMessage) { | ||
| if (result.errorMessage != null && result.errorMessage !== '') { | ||
| showDevError(result.errorMessage) | ||
| return | ||
| } | ||
|
|
@@ -157,18 +162,14 @@ export const ScanModal = (props: Props) => { | |
| logActivity(`QR code read from photo library.`) | ||
| bridge.resolve(response.values[0]) | ||
| }) | ||
| .catch(error => { | ||
| showDevError(error) | ||
| }) | ||
| .catch(showDevError) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. |
||
| } | ||
| ).catch(err => { | ||
| showError(err) | ||
| }) | ||
| ).catch(showError) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. |
||
| } | ||
|
|
||
| const handleClose = () => { | ||
| const handleClose = (): void => { | ||
| triggerHaptic('impactLight') | ||
| // @ts-expect-error | ||
| // @ts-expect-error - AirshipBridge expects string | undefined but resolve() with no args is valid | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In that case, would |
||
| bridge.resolve() | ||
| } | ||
|
|
||
|
|
@@ -186,7 +187,7 @@ export const ScanModal = (props: Props) => { | |
| headerContainerLayout.height + | ||
| (peepholeSpaceLayout.height - holeSize) / 2 | ||
|
|
||
| const renderModalContent = () => { | ||
| const renderModalContent = (): React.JSX.Element | null => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. React.ReactElement is the preferred return type. |
||
| if (!scanEnabled) { | ||
| return null | ||
| } | ||
|
|
@@ -223,7 +224,9 @@ export const ScanModal = (props: Props) => { | |
| style={styles.headerContainer} | ||
| onLayout={handleLayoutHeaderContainer} | ||
| > | ||
| <SceneHeader title={scanModalTitle} underline withTopMargin /> | ||
| {/* This isn't technically a scene, so just using SceneHeaderUi4 directly for simplicity. */} | ||
| {/* eslint-disable-next-line @typescript-eslint/no-deprecated */} | ||
| <SceneHeaderUi4 title={scanModalTitle} /> | ||
| </View> | ||
| <View | ||
| style={[ | ||
|
|
@@ -340,8 +343,8 @@ const getStyles = cacheStyles((theme: Theme) => ({ | |
| }, | ||
| headerContainer: { | ||
| justifyContent: 'flex-end', | ||
| marginBottom: theme.rem(0.5), | ||
| marginTop: theme.rem(1) | ||
| marginTop: theme.rem(2), | ||
| marginLeft: theme.rem(0.5) | ||
| }, | ||
| peepholeSpace: { | ||
| flex: 2 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ import { View } from 'react-native' | |
| import FastImage from 'react-native-fast-image' | ||
| import Animated from 'react-native-reanimated' | ||
| import { useSafeAreaFrame } from 'react-native-safe-area-context' | ||
| import { requestReview } from 'react-native-store-review/src' | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wrong import bypasses review logic, breaks AndroidHigh Severity The Additional Locations (1)
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, what Cursor said. Is this a debugging commit for QA? In that case, just be sure to drop it before merge.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This PR is for the wrong branch - this is the feta test build to allow QA to test easily. |
||
|
|
||
| import { SCROLL_INDICATOR_INSET_FIX } from '../../constants/constantSettings' | ||
| import { ENV } from '../../env' | ||
|
|
@@ -146,6 +147,8 @@ export const HomeScene: React.FC<Props> = props => { | |
| setVideoPosts( | ||
| filterContentPosts(infoServerData.rollup?.videoPosts, countryCode) | ||
| ) | ||
|
|
||
| requestReview() | ||
| }, [countryCode]) | ||
|
|
||
| const buyCryptoIcon = React.useMemo( | ||
|
|
||
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.
Local permission state doesn't sync with Redux updates
Medium Severity
The
cameraPermissionlocal state is initialized fromreduxCameraPermissionbut never syncs with Redux updates after mount. When a user grants camera permission via OS Settings and returns to the app,PermissionsManagerupdates Redux, but the localcameraPermissionstate remains stale. This leaves users stuck on the permission-denied view even after granting permission, requiring them to close and reopen the modal.Additional Locations (1)
src/components/modals/ScanModal.tsx#L90-L101Uh oh!
There was an error while loading. Please reload this page.
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 agree that this is the wrong solution. We don't want to track state locally in the component - that's the job of the permission subsystem. The problem is that the permission subsystem is modifying things and then forgetting to update redux.
Over in the permissions manager, we should do this:
And then here is just becomes:
And presto! The permissions manager is updating redux so this scene works correctly.