Add larivaar settings and save them in Async Storage #32
Conversation
Gauravjeetsingh
commented
Jun 20, 2023
•
Gauravjeetsingh
commented
- Save the larivaar assist and current Ang in storage
- Add a toggle switch for larivaar
… Ang - Also fix the earlier ignored eslint error and few more code optimizations
Gauravjeetsingh
changed the title
| useColorScheme, | ||
| View, | ||
| } from 'react-native'; | ||
| import React, {useEffect, useState} from 'react'; |
There was a problem hiding this comment.
We are not using airbnb eslint style guide. Let's use that veerji
Already in the issues #31
There was a problem hiding this comment.
Will get to this, after all current PRs are merged, and then will add another PR for this.
|
|
||
| useEffect(() => { | ||
| readItemFromStorage(); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps |
There was a problem hiding this comment.
I think we can remove this comment from here and add readItemFromStorage as dependency to fix the issue
There was a problem hiding this comment.
Tried doing that way, but it didn't work. It keeps running the useEffect hook multiple times.
| onValueChange={() => setLarivaarAssist(!larivaarAssist)} | ||
| /> | ||
| <Drawer.Navigator drawerContent={props => getSettings(props)}> | ||
| <Drawer.Screen name="Learn Larivaar" component={Launchpad} /> |
There was a problem hiding this comment.
i18n needs to be added to the project as well. The more we defer i18n, the harder it becomes to implement. You can do that in upcoming PRs
There was a problem hiding this comment.
Hnji veerji, will do.
There was a problem hiding this comment.
Created an issue for this here #36
components/Ang/utils/bakePankti.tsx
Outdated
| export const bakePankti = ( | ||
| verse: string, | ||
| larivaar: Boolean, | ||
| larivaarAssist: Boolean, | ||
| ) => { |
There was a problem hiding this comment.
Recommended to have these arguments in object. Here's the benefit:
- We don't have to firmly abide by the order of arguments while passing in function call
- Extending the number of arguments without worrying about existing code is easy. Reduces the possibility of breaking change in the future.
|
|
||
| useEffect(() => { | ||
| readItemFromStorage(); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps |
There was a problem hiding this comment.
this can be fixed. Similar issue mentioned above
There was a problem hiding this comment.
I tried this, and we even had a call about this, fixing the linting issue, makes this useEffect run on every change, we only want it to run once.
| flexDirection: 'row', | ||
| justifyContent: 'space-between', | ||
| alignItems: 'center', | ||
| marginTop: 16, |
There was a problem hiding this comment.
Don't we need to specify display: 'flex' too?
There was a problem hiding this comment.
display is by default flex here, so not needed.
