Skip to content
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

Minichallenge4 #56

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

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