Challenge 5#3
Conversation
erickwize
left a comment
There was a problem hiding this comment.
Comments
Good work. I left some recommendations.
I see that your tests are not expecting anything, you're only rendering the component, I encourage you to add expect conditions for testing correctly the application and get the certificate.
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
- 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).
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.
| <ProtectedRoute | ||
| path="/favorites" | ||
| isAuthenticated={globalState.authenticated} | ||
| Component={Favorites} | ||
| /> | ||
| <DetailContextProvider> | ||
| <ProtectedRoute | ||
| path="/favoriteDetail" | ||
| isAuthenticated={globalState.authenticated} | ||
| Component={FavoriteDetail} | ||
| /> |
There was a problem hiding this comment.
With the current routing implementation, the user won't be able to share links, because the routing depends on the storage. To handle correctly routing you should use parameters in the path: /favorites/:id and use useParams to obtain the id.
i.e.
import { useParams } from 'react-router-dom';
function Component() {
const { id } = useParams();
}| </ToggleWrapper> | ||
| </ButtonWrapper> | ||
| ); | ||
| return profileButton; |
There was a problem hiding this comment.
I recommend returning the JSX here, it is not adding more readiness with the name of the variable
| function RelatedVideosList({ videos }) { | ||
| console.log('----', videos); | ||
| function RelatedVideosList({ videos, title }) { | ||
| let related = videos |
There was a problem hiding this comment.
Why did you use let? If the value is not going to change, it is recommended to use const so we keep close to the functions without side effects
| <Wrapper> | ||
| <Title>Try a Video</Title> | ||
| <VideosList videos={globalState.videos} /> | ||
| <VideosList videos={globalState.videos} itemPath="/detail" /> |
There was a problem hiding this comment.
I suggest adding the id to the path /detail/<video_id>
This PR contains: