From f6d4fe2e75f975675a51223d829e1213bc973fc0 Mon Sep 17 00:00:00 2001 From: Pablo Velez Vidal Date: Tue, 29 Nov 2022 13:19:02 +0100 Subject: [PATCH] PR feedback refactor --- app/actions/app/global.ts | 14 ++--- app/managers/session_manager.ts | 2 +- app/screens/onboarding/index.tsx | 73 ++++++++++++++------------ app/screens/onboarding/paginator.tsx | 58 ++++++++++---------- app/screens/onboarding/slide.tsx | 11 ++-- app/screens/onboarding/slides_data.tsx | 4 +- 6 files changed, 80 insertions(+), 82 deletions(-) diff --git a/app/actions/app/global.ts b/app/actions/app/global.ts index 2e0df63759..e0c5840c30 100644 --- a/app/actions/app/global.ts +++ b/app/actions/app/global.ts @@ -1,9 +1,9 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import {logError} from '@app/utils/log'; import {GLOBAL_IDENTIFIERS} from '@constants/database'; import DatabaseManager from '@database/manager'; +import {logError} from '@utils/log'; export const storeGlobal = async (id: string, value: unknown, prepareRecordsOnly = false) => { try { @@ -13,6 +13,7 @@ export const storeGlobal = async (id: string, value: unknown, prepareRecordsOnly prepareRecordsOnly, }); } catch (error) { + logError('storeGlobal', error); return {error}; } }; @@ -26,16 +27,7 @@ export const storeMultiServerTutorial = async (prepareRecordsOnly = false) => { }; export const storeOnboardingViewedValue = async (value = true) => { - try { - const {operator} = DatabaseManager.getAppDatabaseAndOperator(); - return operator.handleGlobal({ - globals: [{id: GLOBAL_IDENTIFIERS.ONBOARDING, value}], - prepareRecordsOnly: false, - }); - } catch (error) { - logError('storeOnboardingViewedValue', error); - return {error}; - } + return storeGlobal(GLOBAL_IDENTIFIERS.ONBOARDING, value, false); }; export const storeProfileLongPressTutorial = async (prepareRecordsOnly = false) => { diff --git a/app/managers/session_manager.ts b/app/managers/session_manager.ts index 309c80bd2d..d8d44495cc 100644 --- a/app/managers/session_manager.ts +++ b/app/managers/session_manager.ts @@ -182,7 +182,7 @@ class SessionManager { if (DatabaseManager.appDatabase) { const servers = await queryAllServers(DatabaseManager.appDatabase.database); if (!servers.length) { - storeOnboardingViewedValue(false); + await storeOnboardingViewedValue(false); } } diff --git a/app/screens/onboarding/index.tsx b/app/screens/onboarding/index.tsx index 5971b34ab5..b06919d47e 100644 --- a/app/screens/onboarding/index.tsx +++ b/app/screens/onboarding/index.tsx @@ -1,16 +1,24 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import React, {useCallback, useEffect} from 'react'; -import {View, ListRenderItemInfo, useWindowDimensions, SafeAreaView, ScrollView, StyleSheet, Platform, NativeSyntheticEvent, NativeScrollEvent} from 'react-native'; +import React, {useCallback, useEffect, useRef} from 'react'; +import { + View, + useWindowDimensions, + SafeAreaView, + ScrollView, + StyleSheet, + Platform, + NativeSyntheticEvent, + NativeScrollEvent, +} from 'react-native'; import {Navigation} from 'react-native-navigation'; -import Animated, {useAnimatedRef, useAnimatedStyle, useDerivedValue, useSharedValue, withTiming} from 'react-native-reanimated'; +import Animated, {useAnimatedStyle, useDerivedValue, useSharedValue, withTiming} from 'react-native-reanimated'; import {storeOnboardingViewedValue} from '@actions/app/global'; -import {Screens} from '@app/constants'; +import {Screens} from '@constants'; import Background from '@screens/background'; import {goToScreen, loginAnimationOptions} from '@screens/navigation'; -import {OnboardingItem} from '@typings/screens/onboarding'; import FooterButtons from './footer_buttons'; import Paginator from './paginator'; @@ -45,13 +53,12 @@ const Onboarding = ({ const {width} = useWindowDimensions(); const {slidesData} = useSlidesData(); const LAST_SLIDE_INDEX = slidesData.length - 1; - const dimensions = useWindowDimensions(); - const slidesRef = useAnimatedRef(); + const slidesRef = useRef(null); const scrollX = useSharedValue(0); // used to smothly animate the whole onboarding screen during the appear event scenario (from server screen back to onboarding screen) - const translateX = useSharedValue(dimensions.width); + const translateX = useSharedValue(width); const currentIndex = useDerivedValue(() => Math.round(scrollX.value / width)); @@ -60,6 +67,13 @@ const Onboarding = ({ x: (slideIndexToMove * width), animated: true, }); + }, [width]); + + const signInHandler = useCallback(() => { + // mark the onboarding as already viewed + storeOnboardingViewedValue(); + + goToScreen(Screens.SERVER, '', {animated: true, theme}, loginAnimationOptions()); }, []); const nextSlide = useCallback(() => { @@ -69,31 +83,11 @@ const Onboarding = ({ } else if (slidesRef.current && currentIndex.value === LAST_SLIDE_INDEX) { signInHandler(); } - }, [currentIndex.value, slidesRef.current, moveToSlide]); + }, [currentIndex, slidesRef.current, moveToSlide, signInHandler]); - const signInHandler = useCallback(() => { - // mark the onboarding as already viewed - storeOnboardingViewedValue(); - - goToScreen(Screens.SERVER, '', {animated: true, theme}, loginAnimationOptions()); - }, []); - - const renderSlide = useCallback(({item, index}: ListRenderItemInfo) => { - return ( - - ); - }, [theme]); - - const scrollHandler = (event: NativeSyntheticEvent) => { + const scrollHandler = useCallback((event: NativeSyntheticEvent) => { scrollX.value = event.nativeEvent.contentOffset.x; - }; + }, []); useEffect(() => { const listener = { @@ -101,13 +95,13 @@ const Onboarding = ({ translateX.value = 0; }, componentDidDisappear: () => { - translateX.value = -dimensions.width; + translateX.value = -width; }, }; const unsubscribe = Navigation.events().registerComponentListener(listener, Screens.ONBOARDING); return () => unsubscribe.remove(); - }, [dimensions]); + }, [width]); const transform = useAnimatedStyle(() => { const duration = Platform.OS === 'android' ? 250 : 350; @@ -133,10 +127,19 @@ const Onboarding = ({ pagingEnabled={true} bounces={false} onScroll={scrollHandler} - ref={slidesRef as any} + ref={slidesRef} > {slidesData.map((item, index) => { - return renderSlide({item, index} as ListRenderItemInfo); + return ( + + ); })} ({ - dot: { +const getStyleSheet = makeStyleSheetFromTheme((theme: Theme) => { + const commonStyle = { height: DOT_SIZE / 2, borderRadius: 5, backgroundColor: theme.buttonBg, marginHorizontal: DOT_SIZE / 2, width: DOT_SIZE / 2, opacity: 0.25, - }, - fixedDot: { - height: DOT_SIZE / 2, - borderRadius: 5, - backgroundColor: theme.buttonBg, - marginHorizontal: DOT_SIZE / 2, - width: DOT_SIZE / 2, - opacity: 0.25, - position: 'absolute', - }, - outerDot: { - height: DOT_SIZE, - borderRadius: DOT_SIZE / 2, - backgroundColor: theme.buttonBg, - marginHorizontal: 4, - marginTop: -4, - position: 'absolute', - width: DOT_SIZE, - opacity: 0.15, - }, - paginatorContainer: { - flexDirection: 'row', - height: 15, - justifyContent: 'space-between', - width: 120, - }, -})); + }; + return { + dot: { + ...commonStyle, + }, + fixedDot: { + ...commonStyle, + position: 'absolute', + }, + outerDot: { + height: DOT_SIZE, + borderRadius: DOT_SIZE / 2, + backgroundColor: theme.buttonBg, + marginHorizontal: 4, + marginTop: -4, + position: 'absolute', + width: DOT_SIZE, + opacity: 0.15, + }, + paginatorContainer: { + flexDirection: 'row', + height: 15, + justifyContent: 'space-between', + width: 120, + }, + }; +}); const Paginator = ({ theme, diff --git a/app/screens/onboarding/slide.tsx b/app/screens/onboarding/slide.tsx index 6c22d8fe73..5e5c15edcf 100644 --- a/app/screens/onboarding/slide.tsx +++ b/app/screens/onboarding/slide.tsx @@ -1,14 +1,15 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import React, {useEffect, useState} from 'react'; +import React, {useEffect, useMemo, useState} from 'react'; import {View, useWindowDimensions} from 'react-native'; import Animated, {Extrapolate, interpolate, useAnimatedStyle, useSharedValue, withTiming} from 'react-native-reanimated'; -import {OnboardingItem} from '@typings/screens/onboarding'; import {changeOpacity, makeStyleSheetFromTheme} from '@utils/theme'; import {typography} from '@utils/typography'; +import type {OnboardingItem} from '@typings/screens/onboarding'; + type Props = { item: OnboardingItem; theme: Theme; @@ -114,9 +115,11 @@ const SlideItem = ({theme, item, scrollX, index, lastSlideIndex}: Props) => { }], opacity: initialElementsOpacity.value, }; - });// end of code for animating first image load + }); - const inputRange = [(index - 1) * width, index * width, (index + 1) * width]; + // end of code for animating first image load + + const inputRange = useMemo(() => [(index - 1) * width, index * width, (index + 1) * width], [width, index]); const translateImage = useAnimatedStyle(() => { const translateImageInterpolate = interpolate( diff --git a/app/screens/onboarding/slides_data.tsx b/app/screens/onboarding/slides_data.tsx index 574c13619c..2178535771 100644 --- a/app/screens/onboarding/slides_data.tsx +++ b/app/screens/onboarding/slides_data.tsx @@ -24,7 +24,7 @@ const styles = StyleSheet.create({ }, }); -const useSalidesData = () => { +const useSlidesData = () => { const intl = useIntl(); const theme = useTheme(); const callsSvg = ( @@ -78,4 +78,4 @@ const useSalidesData = () => { return {slidesData}; }; -export default useSalidesData; +export default useSlidesData;