refactor: mentor's review fixes for Dictionary #6
Conversation
AlexeyTeterin
force-pushed
the
refactor-dictionary
branch
from
84c52ed
to
8414e35
Compare
AlexeyTeterin
changed the title
AlexeyTeterin
requested review from
lbratkovskaya,
alexlatypov94,
blackymax,
Gurnick013,
KirillBobkov and
Tonia-SE
| const dispatch = useDispatch(); | ||
| const classes = makeStyles(() => styles)(); | ||
|
|
||
| const fetch = () => dispatch(fetchDictionary()); |
There was a problem hiding this comment.
Имя fetch ни о чём не говорит, хочется больше конкретики, что именно фетчим. Но лучше, чтобы оно не совпадало с fetchDictionary - трудно понимать, где экшн, где метод
There was a problem hiding this comment.
Может, getDictionary тогда?
There was a problem hiding this comment.
Солидно, хотел предложить также
AlexeyTeterin
force-pushed
the
refactor-dictionary
branch
from
8414e35
to
0201aa9
Compare
AlexeyTeterin
force-pushed
the
refactor-dictionary
branch
from
0201aa9
to
85e0651
Compare
| payload: { words }, | ||
| }; | ||
| } | ||
| export const fetchDictionarySuccess = (words: IWord[]) => ({ |
There was a problem hiding this comment.
IWord[] как Array вроде хотели писать?
| }; | ||
| } | ||
| export const fetchDictionary = () => async (dispatch: Dispatch) => { | ||
| const apiURL = 'https://react-learnwords-example.herokuapp.com/words'; |
There was a problem hiding this comment.
в consts вынести нужно, наверное. Или все равно потом другой подставлять...
| state: IDictionary = initialState, | ||
| action: IDictionaryAction | ||
| ) { | ||
| const dictionaryReducer = (state: IDictionary = initialState, action: IDictionaryAction) => { |
There was a problem hiding this comment.
у нас isLoading в AppState переехал
| export const fetchDictionary = () => async (dispatch: Dispatch) => { | ||
| dispatch(fetchDictionaryStart()); | ||
| try { | ||
| const res = await fetch(`${backendUrl}/words`); |
There was a problem hiding this comment.
I think it would be nice if fetchDictionary function will take two value. For example "page" & "group", for this link https://react-learnwords-example.herokuapp.com/words?page=2&group=0
There was a problem hiding this comment.
Будет сделано в будущем ) это пока что "скелет"
| @@ -24,21 +24,19 @@ export interface IWord { | |||
| textExampleTranslate: string; | |||
| } | |||
|
|
|||
| export interface IDictionaryState { | |||
| export interface IWordBookState { | |||
| isLoading: boolean; | |||
| words: IWord[]; | |||
There was a problem hiding this comment.
Вот в таких местах как раз ментором рекомендовано делать Array<IWord>
No description provided.