Skip to content
This repository has been archived by the owner on Feb 9, 2024. It is now read-only.

London10 - Khalil Alhaydr - CYF-hotel-challenge #614

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Khlil1313
Copy link

No description provided.

@sonarcloud
Copy link

sonarcloud bot commented Jul 8, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

Copy link

@mira-shukla mira-shukla left a comment

Choose a reason for hiding this comment

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

Really great work! only some small ideas/changes.

<div className="CityDives">
<TouristInfoCards cityId={0} />
<TouristInfoCards cityId={1} />
<TouristInfoCards cityId={2} />

Choose a reason for hiding this comment

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

Is it possible to move these cityIds into a variable rather than each one individually?

<div>
<header className="App-header">CYF Hotel</header>;
<img className="Logo-image" src="https://images.unsplash.com/photo-1615880484746-a134be9a6ecf?ixlib=rb-4.0.3&ixid=M3wxMjA3fDB8MHxzZWFyY2h8M3x8bW91bnRhaW4lMjBob3RlbHxlbnwwfHwwfHx8MA%3D%3D&auto=format&fit=crop&w=500&q=60"></img>
</div>

Choose a reason for hiding this comment

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

Great work, sometimes it is good practice to add 'alt' incase the image doesn't load. More info here.)

@@ -0,0 +1,42 @@
import React from "react";

const city = [

Choose a reason for hiding this comment

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

Small thing but up to you it may be better to rename this to 'cities' as it is an array of cities.

return (
<div className="CitiesDiv">
<div className="card">
<img src={city[cityId].src} className="card-img-top" />

Choose a reason for hiding this comment

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

is it possible to restructure city[cityId] so it is called from another level?

@mira-shukla mira-shukla added the reviewed A volunteer has reviewed this PR label Jul 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
reviewed A volunteer has reviewed this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants