-
Notifications
You must be signed in to change notification settings - Fork 18
Complete Project Overhaul: Responsive Design, Actor Detail Modal, and Unit tests #5
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
base: main
Are you sure you want to change the base?
Changes from all commits
9acf837
f09d1f8
efcdef1
4ff72ac
110e7cf
7c67052
b05f5b5
ff405d3
e7474e1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,19 +1,52 @@ | ||
| Contains React.Js code challenge for internship applicants to Mereb Technologies. Please carefully follow the instructions given below. | ||
| # Star Wars Actors | ||
|
|
||
| **Create Simple React.Js application** | ||
| ## Description | ||
|
|
||
| Your task is to create react.js application and fetch a list of actors with the films they have been involved from a given API endpoint and render the list of actors on the page. Each actor should be displayed as a card with its name, height,birth_year and an "Detail" button. | ||
| Star Wars Actors is a React-based application that fetches and displays a list of actors from the Star Wars universe. Users can click on an actor's card to view detailed information, including their attributes and film appearances. | ||
|
|
||
| **Details:** | ||
| ## Features | ||
|
|
||
| 1. Fetch the product data from the following API endpoint: https://swapi.py4e.com/api/people/. | ||
| - Fetches actor data from an API. | ||
| - Displays actors in a responsive layout. | ||
| - Allows users to view detailed information about each actor. | ||
| - Interactive design with a close button to hide details. | ||
|
|
||
| 2. Display the list of actors as cards. Each card should include the product name, height,birth_year and an "Detail" button. | ||
| ## Technologies Used | ||
|
|
||
| 3. When the "Detail" button is clicked, the selected actor should be displayed in separated component with the detail of an actor. | ||
| - **React**: JavaScript library for building user interfaces. | ||
| - **Redux Toolkit**: For state management. | ||
| - **React Testing Library**: For unit testing components. | ||
| - **CSS**: For styling the application. | ||
|
|
||
| 4. Use appropriate CSS styles to make the actor list visually appealing. | ||
| ## Screenshots | ||
|
|
||
| 5. Errors and loading states should be handled. | ||
| 6. Write unit tests to ensure that the components behave as expected. | ||
|
|
||
| Here are some screenshots of the application for all sizes(large , medium and small): | ||
|
|
||
| # Desktop | ||
|
|
||
|  | ||
|
|
||
| # Tablet | ||
|
|
||
|  | ||
|
|
||
| # Mobile | ||
|
|
||
|  | ||
|
|
||
| # Detail Component | ||
|
|
||
|  | ||
|
|
||
| ## Installation | ||
|
|
||
| 1. Clone the repository: | ||
| git clone https://github.com/HanaGt/React-Challenge.git | ||
|
|
||
| 2. Navigate to the project directory: | ||
| cd React-Challenge | ||
| 3. Install the dependencies: | ||
| npm install | ||
| 4. Start the application: | ||
| npm start | ||
| 5. Open your browser and visit http://localhost:3000 to view the application. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,12 +3,17 @@ | |
| "version": "0.1.0", | ||
| "private": true, | ||
| "dependencies": { | ||
| "@testing-library/jest-dom": "^5.16.5", | ||
| "@testing-library/react": "^13.4.0", | ||
| "@reduxjs/toolkit": "^2.2.7", | ||
| "@testing-library/dom": "^10.4.0", | ||
| "@testing-library/jest-dom": "^6.5.0", | ||
| "@testing-library/react": "^16.0.1", | ||
| "@testing-library/user-event": "^13.5.0", | ||
| "react": "^18.2.0", | ||
| "react-dom": "^18.2.0", | ||
| "react-redux": "^9.1.2", | ||
| "react-scripts": "5.0.1", | ||
| "redux": "^5.0.1", | ||
| "redux-toolkit": "^1.1.2", | ||
|
Comment on lines
+15
to
+16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove unnecessary redux-toolkit package The addition of
Please remove the |
||
| "web-vitals": "^2.1.4" | ||
| }, | ||
| "scripts": { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| .App { | ||
| /* .App { | ||
| text-align: center; | ||
| } | ||
|
|
||
|
|
@@ -35,4 +35,4 @@ | |
| to { | ||
| transform: rotate(360deg); | ||
| } | ||
| } | ||
| } */ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,25 +1,20 @@ | ||
| import logo from './logo.svg'; | ||
| import './App.css'; | ||
| import React from 'react'; | ||
| import { Provider } from 'react-redux'; | ||
| import { store } from './api/store'; | ||
| import ActorList from './components/ActorList'; | ||
|
|
||
| // import './App.css'; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure necessary styles are included for the 'App' component You have commented out the import statement for Option 1: Uncomment the import statement to include the styles: -// import './App.css';
+import './App.css';Option 2: Remove the - <div className="App">
+ <div>Also applies to: 11-11 |
||
|
|
||
| function App() { | ||
| return ( | ||
| <div className="App"> | ||
| <header className="App-header"> | ||
| <img src={logo} className="App-logo" alt="logo" /> | ||
| <p> | ||
| Edit <code>src/App.js</code> and save to reload. | ||
| </p> | ||
| <a | ||
| className="App-link" | ||
| href="https://reactjs.org" | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| > | ||
| Learn React | ||
| </a> | ||
| </header> | ||
| </div> | ||
| <Provider store={store}> | ||
| <div className="App"> | ||
| <ActorList /> | ||
| </div> | ||
| </Provider> | ||
| ); | ||
| } | ||
|
|
||
| export default App; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,9 @@ | ||
| import React from 'react'; | ||
| import { render, screen } from '@testing-library/react'; | ||
| import App from './App'; | ||
| import App from './App' | ||
|
|
||
| test('renders learn react link', () => { | ||
| test('renders loading message', () => { | ||
| render(<App />); | ||
| const linkElement = screen.getByText(/learn react/i); | ||
| expect(linkElement).toBeInTheDocument(); | ||
| const loadingElement = screen.getByText(/loading actors.../i); | ||
| expect(loadingElement).toBeInTheDocument(); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| import React from 'react'; | ||
| import { render, screen, fireEvent } from '@testing-library/react'; | ||
| import '@testing-library/jest-dom'; | ||
| import ActorDetail from '../components/ActorDetail'; | ||
|
|
||
| describe('ActorDetail Component', () => { | ||
| const mockActor = { | ||
| name: 'Luke Skywalker', | ||
| height: '172', | ||
| mass: '77', | ||
| hair_color: 'blond', | ||
| skin_color: 'fair', | ||
| eye_color: 'blue', | ||
| birth_year: '19BBY', | ||
| gender: 'male', | ||
| homeworld: 'https://swapi.dev/api/planets/1/', | ||
| films: ['Film 1', 'Film 2'], | ||
| vehicles: ['Vehicle 1'], | ||
| starships: ['Starship 1'] | ||
| }; | ||
|
|
||
| it('should call the onClose handler when the close button is clicked', () => { | ||
| const onCloseMock = jest.fn(); | ||
| render(<ActorDetail actor={mockActor} onClose={onCloseMock} />); | ||
|
|
||
| fireEvent.click(screen.getByText('Close')); | ||
|
|
||
| expect(onCloseMock).toHaveBeenCalled(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| import React from 'react'; | ||
| import { render, screen } from '@testing-library/react'; | ||
| import '@testing-library/jest-dom'; | ||
| import ActorList from '../components/ActorList'; | ||
| import { useFetchActorsQuery } from '../api/slices/actorSlice'; | ||
|
|
||
|
|
||
| jest.mock('../api/slices/actorSlice', () => ({ | ||
| useFetchActorsQuery: jest.fn(), | ||
| })); | ||
|
|
||
| describe('ActorList Component', () => { | ||
| it('should display loading message while fetching actors', () => { | ||
| useFetchActorsQuery.mockReturnValue({ | ||
| data: null, | ||
| error: null, | ||
| isLoading: true, | ||
| }); | ||
|
|
||
| render(<ActorList />); | ||
|
|
||
| expect(screen.getByText('Loading actors...')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('should display error message if fetching fails', () => { | ||
| useFetchActorsQuery.mockReturnValue({ | ||
| data: null, | ||
| error: { message: 'Failed to fetch actors' }, | ||
| isLoading: false, | ||
| }); | ||
|
|
||
| render(<ActorList />); | ||
|
|
||
| expect(screen.getByText('Error loading actors: Failed to fetch actors')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('should display actors when data is fetched successfully', () => { | ||
| useFetchActorsQuery.mockReturnValue({ | ||
| data: { | ||
| results: [ | ||
| { name: 'Hana Guta', height: '165', mass: '55' }, | ||
| { name: 'John Doe', height: '202', mass: '136' }, | ||
| ], | ||
| }, | ||
| error: null, | ||
| isLoading: false, | ||
| }); | ||
|
|
||
| render(<ActorList />); | ||
|
|
||
| expect(screen.getByText('Hana Guta')).toBeInTheDocument(); | ||
| expect(screen.getByText('John Doe')).toBeInTheDocument(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| import { createApi, fetchBaseQuery } from '@reduxjs/toolkit/query/react'; | ||
| import { createSlice } from '@reduxjs/toolkit'; | ||
|
|
||
|
|
||
| export const actorsApi = createApi({ | ||
| reducerPath: 'actorsApi', | ||
| baseQuery: fetchBaseQuery({ baseUrl: 'https://swapi.py4e.com/api/' }), | ||
| endpoints: (builder) => ({ | ||
| fetchActors: builder.query({ | ||
| query: () => 'people/', | ||
| }), | ||
| }), | ||
| }); | ||
|
|
||
| export const { useFetchActorsQuery } = actorsApi; | ||
|
|
||
| const actorsSlice = createSlice({ | ||
| name: 'actors', | ||
| initialState: { | ||
| actors: [], | ||
| status: 'idle', | ||
| }, | ||
| reducers: { | ||
| setActors: (state, action) => { | ||
| state.actors = action.payload; | ||
| }, | ||
| setStatus: (state, action) => { | ||
| state.status = action.payload; | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| export const { setActors, setStatus } = actorsSlice.actions; | ||
|
|
||
| export const selectActors = (state) => state.actors.actors; | ||
| export const selectStatus = (state) => state.actors.status; | ||
|
|
||
| export default actorsSlice.reducer; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import { configureStore } from '@reduxjs/toolkit'; | ||
| import { actorsApi } from './slices/actorSlice' | ||
|
|
||
| export const store = configureStore({ | ||
| reducer: { | ||
| [actorsApi.reducerPath]: actorsApi.reducer, | ||
| }, | ||
| middleware: (getDefaultMiddleware) => | ||
| getDefaultMiddleware().concat(actorsApi.middleware), | ||
| }); | ||
|
Comment on lines
+4
to
+10
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider adding error handling and environment-specific configuration. While the current setup is functional, consider enhancing it with the following:
Example: const isDevelopment = process.env.NODE_ENV === 'development';
export const store = configureStore({
reducer: {
[actorsApi.reducerPath]: actorsApi.reducer,
},
middleware: (getDefaultMiddleware) =>
getDefaultMiddleware().concat(actorsApi.middleware),
devTools: isDevelopment, // Enable Redux DevTools only in development
});
if (isDevelopment) {
console.log('Redux store initialized in development mode');
}This enhancement will improve debugging capabilities and ensure appropriate behavior across different environments. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| import React from 'react'; | ||
| import './ActorList.css'; | ||
|
|
||
| const ActorCard = ({ actor , onSelect}) => { | ||
| return ( | ||
| <div className="actor-card"> | ||
| <div className="actor-info"> | ||
| <img | ||
| src={`https://ui-avatars.com/api/?name=${actor.name}&background=random`} | ||
| alt={actor.name} | ||
| className="actor-image" | ||
| /> | ||
| <div className="actor-details"> | ||
| <p className="actor-name">{actor.name}</p> | ||
| <p className="actor-role">DOB:{actor.birth_year}</p> | ||
| <p className="actor-role">Height: {actor.height}</p> | ||
|
|
||
|
Comment on lines
+4
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider adding PropTypes and default props. While the props are used correctly, adding PropTypes and default props would improve the component's robustness and documentation. Consider adding PropTypes and default props like this: import PropTypes from 'prop-types';
// ... component code ...
ActorCard.propTypes = {
actor: PropTypes.shape({
name: PropTypes.string.isRequired,
birth_year: PropTypes.string,
height: PropTypes.string,
}).isRequired,
onSelect: PropTypes.func.isRequired,
};
ActorCard.defaultProps = {
actor: {
name: 'Unknown Actor',
birth_year: 'Unknown',
height: 'Unknown',
},
onSelect: () => {},
};Don't forget to import PropTypes at the top of the file: import PropTypes from 'prop-types'; |
||
| </div> | ||
| </div> | ||
| <button className="actor-button" onClick={onSelect}> | ||
| Details | ||
| </button> | ||
|
Comment on lines
+20
to
+22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider adding visual feedback for button interaction. The "Details" button correctly uses the Consider adding hover and active states to the button in your CSS: .actor-button:hover {
background-color: #e0e0e0;
}
.actor-button:active {
background-color: #d0d0d0;
}Also, you might want to add a loading state to the button when the details are being fetched: <button
className={`actor-button ${isLoading ? 'loading' : ''}`}
onClick={onSelect}
disabled={isLoading}
>
{isLoading ? 'Loading...' : 'Details'}
</button>This would require adding an |
||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| export default ActorCard; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| .actor-detail { | ||
| position: fixed; | ||
| top: 50%; | ||
| left: 50%; | ||
| transform: translate(-50%, -50%); | ||
| width: 90%; | ||
| max-width: 400px; | ||
| background-color: #202040; | ||
| padding: 20px; | ||
| border-radius: 15px; | ||
| color: white; | ||
| z-index: 10; | ||
| box-shadow: 0 2px 10px rgba(0, 0, 0, 0.2); | ||
| overflow: auto; | ||
| } | ||
|
|
||
| .actor-detail h2 { | ||
| margin-bottom: 15px; | ||
| font-size: 1.5rem; | ||
| } | ||
|
|
||
| .actor-detail p { | ||
| margin: 5px 0; | ||
| font-size: 1rem; | ||
| } | ||
|
|
||
| .close-button { | ||
| background-color: #a72bcf; | ||
| border: none; | ||
| border-radius: 10px; | ||
| padding: 8px 15px; | ||
| font-size: 0.9rem; | ||
| color: white; | ||
| cursor: pointer; | ||
| float: right; | ||
| } | ||
|
|
||
| .close-button:hover { | ||
| background-color: #c92bcf; | ||
| } | ||
|
|
||
|
|
||
| @media (max-width: 768px) { | ||
| .actor-detail { | ||
| padding: 15px; | ||
| } | ||
|
|
||
| .actor-detail h2 { | ||
| font-size: 1.3rem; | ||
| } | ||
|
|
||
| .actor-detail p { | ||
| font-size: 0.9rem; | ||
| } | ||
|
|
||
| .close-button { | ||
| padding: 6px 10px; | ||
| font-size: 0.8rem; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great addition of screenshots, but image links need updating
The Screenshots section is a valuable addition to the README, showcasing the application's responsive design and the new Actor Detail Modal. This aligns well with the PR objectives.
However, the image links are currently using placeholder text (e.g.,
). Please update these links to point to the actual screenshot files in your repository. For example:Ensure that the image files are committed to the repository and that the paths are correct.