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

Mini ch 5 #66

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

Mini ch 5 #66

wants to merge 43 commits into from

Conversation

cylcrow
Copy link

@cylcrow cylcrow commented Mar 26, 2021

What this PR do?

Performs tasks for mini challenge 5

Any background context you want to provide?

⚠️ ⚠️ ⚠️ ⚠️ ⚠️ Lots of new features, tests and components. 🚀 🧪 : 🔥

How should this be manually tested?

running tests
Or enter here

Screenshots

image
image
image
image
image
image
image
image
image

Test coverage
image

NOTE: If videos doesn't load it could be due to exceeded YouTube Data API quota.

OMG! 😃

cylcrow and others added 30 commits February 19, 2021 16:16
- Removed unnecessary files
- Added first tests
- Added first components
- Added dependencies for testing
- First iteration for base components
- Modified Header structure: created StyledSection component to a better arrangement of components
- Added card component
- Added HomeVideos component
- Added tests for new added components
- Added/refactored styles for HomeVideo and Card component
- Refactored tests for Card component
…ests refactor to comply just with react-testing-library
- Added roles to elements to improve accesibility
- Refactored tests to find elements by role instead of their id
- ✨ Added theming prototype
- ✅ Added first specs to test integration with theming
- Changed directory name 'Base' to 'ui', indicating that files contained here will be for general UI integration
- Updated tests in Card component
- Refactored code
- Moved mocked files to a new folder
- Mocked API calls
- Created tests for custom hooks
- Added test for YT playback frame
- Added component for youtube playback
- Added SmallVideoCard component
- Added RelatedVideoList component
- Added styles for each new component
- Added tests for each new component
- Removed unnecesary code from VideoPlayerContainer
- Added tests to assert small captions video change
- Improved code format
- Added AppContext to share global context
- Moved theme spec to AppContext
- Moved useSearch to App.jsx component
- Added appReducer for global context: AppContext
- Deleted unnecessary files
- Added test for useReducer, googleMockedAPIObject files
- Prevented props passing from parent componenst to child componenst using context props
- Reformated code on some files
- Added tests for loginService
- Added tests for login screen
- Added test for functionality to redirect to /home on successful login
@cylcrow cylcrow force-pushed the mini-ch-5 branch 2 times, most recently from c94c67e to 77ab3cb Compare March 29, 2021 21:10
@cylcrow cylcrow force-pushed the mini-ch-5 branch 4 times, most recently from f94b608 to 5b8c5b2 Compare March 30, 2021 19:43
- Context value passed to hide/show favorite buttons
- Added routes for logged user
Copy link

@jparciga jparciga left a comment

Choose a reason for hiding this comment

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

Mini-Challenge 5 Feedback

Excellent job Andrés 👏🏻👏🏻👏🏻 !!! You applied all the concepts covered during the Bootcamp and even more!! Your code is very well structured and organized, you really followed a TDD approach in your project and you tested almost every single functionality and use-case, I'm really impressed and I'm pretty sure that you already are and continue to be an excellent engineer, congratulations 🙌🏻🎉🍾 !!

Acceptance Criteria

✅ All the sections have their own route.
✅ Navigation across the sections is implemented correctly.
✅ The Global State is persistent across all the sections.
✅ The "Mocked Authentication" mechanism works correctly.
✅ The "session data" is stored in the Global Context correctly.
✅ Videos can be added to the Favorites list.
✅ Videos can be removed from the Favorites list.
✅ Navigation to Favorite Videos View using a private route is implemented correctly (only authenticated users should access this section).
✅ Navigation to the Favorite Video Details View using a private route is implemented correctly (only authenticated users should access this section).
½ Information for the selected favorite video is displayed correctly -> Video details information is missing underneath the player:
Screen Shot 2021-03-30 at 18 18 41
✅ The list of other favorite videos in the Favorite Video Details View is displayed.
✅ 🙌🏻. Testing coverage is above 70%. (Please include a screenshot of the code coverage report in your PR description). -> Awesome job implementing almost every test-case possible for your components, reducers, utils, etc.

Screen Shot 2021-03-30 at 15 57 05
Just a few tests failed, but don't worry you already achieved the desired coverage for this demo project :)
Screen Shot 2021-03-30 at 15 57 48

Bonus Points

✅ The Add/remove from favorites button should appear when the user passes the mouse over the video card in the list.
✅ Integrate with a real authentication provider (such as Auth0, OAuth, or Firebase).
✅ A 404 section is shown when a route is not found.
✅ The Login View is implemented as a modal using React Portals.

Comment on lines 34 to 46
it('fires user search until presses enter', async () => {
const EXPECTED_TEXT = 'Hello, ';
const TRIGGER_TYPING = 'there';
const { searchInput, contextValue } = build();
const built = await build();
const { searchInput, history, contextValue } = built;
fireEvent.change(searchInput(), { target: { value: TRIGGER_TYPING } });
fireEvent.change(searchInput(), { target: { value: EXPECTED_TEXT } });
expect(searchInput().value).toBe(EXPECTED_TEXT);
expect(contextValue.search).not.toHaveBeenCalled();
fireEvent.keyPress(searchInput(), { key: 'Enter', code: 13, charCode: 13 });
expect(history().location.pathname).toBe("/home");
expect(contextValue.search).toHaveBeenCalled();
});

Choose a reason for hiding this comment

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

Nice test case 👌🏻

Comment on lines 16 to 48
describe('Button', () => {

it('Shows specified text', () => {
const EXPECTED_TEXT = 'Test text'
const { firstChild } = build(<Button>{EXPECTED_TEXT}</Button>).container;
expect(firstChild).toHaveTextContent(EXPECTED_TEXT);
})

it('Has passed id', () => {
const EXPECTED_ID = 'button-id';
const { firstChild } = build(<Button id={EXPECTED_ID}/>).container;
expect(firstChild).toHaveProperty('id', EXPECTED_ID);
})

it('renders with light theme', () => {
const { firstChild } = build().container;
expect(firstChild).toHaveStyle(`background: ${lightTheme.color.secondary}`);
expect(firstChild).toHaveStyle(`color: ${lightTheme.color.fontPrimary}`);
});

it('renders with dark theme', () => {
const { firstChild } = build(<Button/>, darkTheme).container;
expect(firstChild).toHaveStyle(`background: ${darkTheme.color.secondary}`);
expect(firstChild).toHaveStyle(`color: ${darkTheme.color.fontPrimary}`);
});

it('triggers onClick', () => {
const mockedFunction = jest.fn();
const { firstChild } = build(<Button onClick={mockedFunction}/>).container;
fireEvent.click(firstChild);
expect(mockedFunction).toHaveBeenCalledTimes(1);
});

Choose a reason for hiding this comment

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

Great way to test your component 👍🏻

Comment on lines +17 to +51
it('mutates "sidebarOpen" only', () => {
const EXPECTED_VALUE = true;
const passedState = deepCopy(initialState);
const stateResult = headerReducer(passedState, { type: SET_SIDEBAR_OPEN, payload: EXPECTED_VALUE });
expect(stateResult.sidebarOpen).toBe(EXPECTED_VALUE);
expect(stateResult.avatarMenuOpen).toBe(passedState.avatarMenuOpen);
expect(stateResult.loginFormOpen).toBe(passedState.loginFormOpen);
});

it('mutates "avatarMenuOpen" only', () => {
const EXPECTED_VALUE = true;
const passedState = deepCopy(initialState);
const stateResult = headerReducer(passedState, { type: SET_AVATAR_MENU_OPEN, payload: EXPECTED_VALUE });
expect(stateResult.sidebarOpen).toBe(passedState.sidebarOpen);
expect(stateResult.avatarMenuOpen).toBe(EXPECTED_VALUE);
expect(stateResult.loginFormOpen).toBe(passedState.loginFormOpen);
});

it('mutates "loginFormOpen" only', () => {
const EXPECTED_VALUE = true;
const passedState = deepCopy(initialState);
const stateResult = headerReducer(passedState, { type: SET_LOGIN_FORM_OPEN, payload: EXPECTED_VALUE });
expect(stateResult.sidebarOpen).toBe(passedState.sidebarOpen);
expect(stateResult.avatarMenuOpen).toBe(passedState.avatarMenuOpen);
expect(stateResult.loginFormOpen).toBe(EXPECTED_VALUE);
});

it('Does not mutate anyting when action type is not recognized', () => {
const EXPECTED_VALUE = true;
const passedState = deepCopy(initialState);
const stateResult = headerReducer(passedState, { type: '?', payload: EXPECTED_VALUE });
expect(stateResult.sidebarOpen).toBe(passedState.sidebarOpen);
expect(stateResult.avatarMenuOpen).toBe(passedState.avatarMenuOpen);
expect(stateResult.loginFormOpen).toBe(passedState.loginFormOpen);
});

Choose a reason for hiding this comment

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

Awesome!

Comment on lines +38 to +98
it('applies "light" theme if none selected', async () => {
const wrapper = await build();
expect(wrapper.LayoutWrapper()).toHaveStyle(
`background: ${lightTheme.color.background}`
);
});

it('changes "light" theme to "dark" theme', async () => {
const wrapper = await build();

expect(wrapper.LayoutWrapper()).toHaveStyle(
`background: ${lightTheme.color.background}`
);
await act(async () => {
fireEvent.click(wrapper.ThemeSwitch());
});
expect(wrapper.LayoutWrapper()).toHaveStyle(
`background: ${darkTheme.color.background}`
);
});

it('shows videos list after successful fetch', async () => {
const built = (await build());
const { videosList } = built;
expect(videosList()).toHaveLength(youtubeMockedData.items.length);
});

it('shows "No hay videos :/" legend after failed fetch', async () => {
global.gapi = googleMockedAPIObject(false);
const built = (await build());
const { noVideosAvailableCaption } = built;
expect( noVideosAvailableCaption() ).toBeTruthy();
});

it('waits until `gapi` object is defined before first search', async () => {
global.gapi = undefined;
const built = await build();

await for400Miliseconds();
global.gapi = googleMockedAPIObject();
await act( async () => { await for400Miliseconds(); });

const { videosList } = built;
expect(videosList()).toHaveLength(youtubeMockedData.items.length);
});

it('shows "Couldn\'t load videos" when "gapi" object is not loaded after 1 second', async () => {
global.gapi = undefined;
const built = await build();

await for400Miliseconds();
await for400Miliseconds();

await act( async () => {
await for400Miliseconds();
global.gapi = googleMockedAPIObject();
});

const { noVideosAvailableCaption } = built;
expect(noVideosAvailableCaption()).toBeTruthy();
});

Choose a reason for hiding this comment

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

Excellent 👏🏻

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