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

London10-Olha_Danylevska-React_Module-CYF_hotel_react #601

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

Conversation

OlhaDanylevska
Copy link

No description provided.

Copy link

@sherif98 sherif98 left a comment

Choose a reason for hiding this comment

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

Great work Olha!

Comment on lines +49 to +63
<div className="AllCards">

<TouristInfoCards name="Manchester" src="https://cdn.britannica.com/42/116342-050-5AC41785/Manchester-Eng.jpg"
link="https://www.visitmanchester.com/" description="Manchester is the only UK city to feature in Lonely Planet's Best in Travel 2023 list and the only
UK city in National Geographic's influential ‘Best of the World’ list which annually sets out 25 of the must-see places to visit around the globe." />

<TouristInfoCards name="London" src="https://images.musement.com/cover/0002/49/london-jpeg_header-148518.jpeg"
link="https://www.visitlondon.com/" description="Welcome to London! Discover the best of London with Visit London, the official guide to England’s
exciting capital. Find things to do in London, from iconic sightseeing spots and fun-filled days out to top restaurants, theatre and unmissable London events. If you’re not able to visit just yet, plan ahead to make the most of your next visit." />

<TouristInfoCards name={"Glasgow"} src="https://www.visitscotland.com/blog/wp-content/uploads/2021/10/Park_Circus_and_Kelvingrove_Park.jpg.jpg"
link="https://peoplemakeglasgow.com/" description="You’re guaranteed to find accommodation in Glasgow which suits your taste and budget. Whether you’re looking for something uniquely Glaswegian, a trendy hotel, a vibrant hostel or
a comfortable city centre apartment, you can be assured of a warm welcome." />

</div>

Choose a reason for hiding this comment

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

The TouristInfoCards component represents one card, yet it's plural (i.e. Cards not Card). I think it's better to have two components, first is TouristInfoCards which uses multiple TouristInfoCard components.

For example:

TouristInfoCards () =>
return (
TouristInfoCard(name = {"Glasgow"}
TouristInfoCard(name = {"London"}
etc.
)

You can also take the TouristInfoCards component to a new file to keep the App.js file simple and clean.

src/Bookings.js Outdated

})
.then((data) => {
console.log("data----->", data)

Choose a reason for hiding this comment

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

nit: It's usually better to remove the debugging statements before you send your code for review.

Comment on lines 28 to 29
let a = moment(client.checkOutDate)
let b = moment(client.checkInDate)

Choose a reason for hiding this comment

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

nit: it's better to have a more descriptive variable names.

let checkOutDate = moment(client.checkOutDate)
let checkInDate = moment(client.checkInDate)

Copy link
Author

Choose a reason for hiding this comment

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

thanks will do)

src/App.js Outdated
Comment on lines 7 to 30
const Heading = () => {
return (
<div>
<header className="App-header">CYF Hotel</header>
</div>
)
}

const Footer = (props) => {
let data = props.address
return (
<div className="footer">
<ul>
{
data.map(line => {
return <li>{line}</li>
})
}
</ul>
</div>
)


}

Choose a reason for hiding this comment

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

What do you think of moving the Heading and Footer components in two separate files, Heading.js and Footer.js. It'll be more future proof in case the components get larger and require more work.

@migmow migmow added the reviewed A volunteer has reviewed this PR label Jul 10, 2023
@sonarqubecloud
Copy link

sonarqubecloud bot commented Aug 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

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.

3 participants