Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 44 additions & 11 deletions README.md
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

![alt text](image.png)

# Tablet

![alt text](tablet.jpg)

# Mobile

![alt text](mobile.jpg)

# Detail Component

![alt text](image-1.png)
Comment on lines +21 to +39
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue

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., ![alt text](image.png)). Please update these links to point to the actual screenshot files in your repository. For example:

![Desktop View](./screenshots/desktop.png)
![Tablet View](./screenshots/tablet.jpg)
![Mobile View](./screenshots/mobile.jpg)
![Detail Component](./screenshots/detail-component.png)

Ensure that the image files are committed to the repository and that the paths are correct.


## 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.
Binary file added image-1.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added image.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added mobile.jpg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
9 changes: 7 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove unnecessary redux-toolkit package

The addition of redux (version 5.0.1) is appropriate and aligns with the PR objectives. However, the redux-toolkit package (version 1.1.2) is unnecessary and potentially problematic:

  1. You've already included @reduxjs/toolkit, which is the official, up-to-date package for Redux Toolkit.
  2. redux-toolkit is an older, unofficial package that's no longer maintained.

Please remove the redux-toolkit package to avoid potential conflicts and confusion. You can do this by running:

npm uninstall redux-toolkit

"web-vitals": "^2.1.4"
},
"scripts": {
Expand Down
4 changes: 2 additions & 2 deletions src/App.css
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
.App {
/* .App {
text-align: center;
}

Expand Down Expand Up @@ -35,4 +35,4 @@
to {
transform: rotate(360deg);
}
}
} */
31 changes: 13 additions & 18 deletions src/App.js
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';
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure necessary styles are included for the 'App' component

You have commented out the import statement for App.css on line 6, but the <div> on line 11 still has className="App". This may lead to missing styles in your application. Consider one of the following options:

Option 1: Uncomment the import statement to include the styles:

-// import './App.css';
+import './App.css';

Option 2: Remove the className if the styles are no longer needed:

-      <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;


9 changes: 5 additions & 4 deletions src/App.test.js
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();
});
30 changes: 30 additions & 0 deletions src/__tests__/ActorDetail.test.js
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();
});
});
54 changes: 54 additions & 0 deletions src/__tests__/ActorList.test.js
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();
});
});
38 changes: 38 additions & 0 deletions src/api/slices/actorSlice.js
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;
10 changes: 10 additions & 0 deletions src/api/store.js
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
Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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:

  1. Error handling: Implement a mechanism to catch and handle potential errors during store creation or API calls.
  2. Environment-specific configuration: Add conditional logic to adjust store settings based on the current environment (development, production, etc.).

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.

27 changes: 27 additions & 0 deletions src/components/ActorCard.js
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
Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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
Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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 onSelect prop for its onClick handler. However, adding visual feedback when the button is interacted with could improve user experience.

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 isLoading prop to the component and updating the PropTypes accordingly.

</div>
);
};

export default ActorCard;
60 changes: 60 additions & 0 deletions src/components/ActorDetail.css
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;
}
}
Loading