Skip to content

Conversation

@AlanCantabrana
Copy link

Captura de Pantalla 2021-04-14 a la(s) 23 32 28

Copy link

@PacoMojica PacoMojica left a comment

Choose a reason for hiding this comment

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

Hi Alan, you already have some things required for the next delivery, your challenge is looking good, great work!

Acceptance Criteria

  • The search term is stored and retrieved from the Global Context correctly.
  • The appearance theme is stored on the Global Context and applied correctly to the App UI.
  • useReducer hook is implemented correctly to manage the Global State.

Bonus Points

  • Testing coverage is above 70%. (Please include a screenshot of the code coverage report in your PR description).

Comment on lines +5 to +30
const GlobalContext = createContext();

const useGlobalProvider = () => {
const context = useContext(GlobalContext);
if (!context) {
throw new Error(`Can't use "useGlobal" without an GlobalProvider!`);
}
return context;
};

function GlobalProvider({ children }) {
const [state, dispatch] = useReducer(globalReducer, initialState);

useEffect(() => {
window.localStorage.setItem('theme', state.themeValue);
}, [state.themeValue]);

return (
<GlobalContext.Provider value={{ state, dispatch }}>
{children}
</GlobalContext.Provider>
);
}

export { useGlobalProvider, GlobalContext };
export default GlobalProvider;

Choose a reason for hiding this comment

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

Awesome, the actions and the reducer is well defined, good stuff

Comment on lines +4 to +10
SWITCH_THEME: 'SWITCH_THEME',
UPDATE_SEARCH_VALUE: 'UPDATE_SEARCH_VALUE',
SELECT_VIDEO: 'SELECT_VIDEO',

GET_VIDEOS_REQUEST: 'GET_VIDEOS_REQUEST',
GET_VIDEOS_SUCCESS: 'GET_VIDEOS_SUCCESS',
GET_VIDEOS_FAILURE: 'GET_VIDEOS_FAILURE',

Choose a reason for hiding this comment

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

I like that you defined the actions as constants 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants