Skip to content
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

. #327

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

. #327

wants to merge 34 commits into from

Conversation

AnnaElvine
Copy link

my page

Copy link

@emiliasaberski emiliasaberski left a comment

Choose a reason for hiding this comment

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

I think you've done a great job with really nice animations and effects! I see that you've even started on the dropdown, hope you get time to incorporate that as well! There are only some small adjustments that needs to be done to the styling on the detail-page. Well fought this week 💪🏻☀️

}
// eslint-disable-next-line no-unused-vars
const handleBackButton = (event) => {
if (event.KeyCode === 13) {

Choose a reason for hiding this comment

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

What does this mean? Should I be able to press Enter on the backbutton?

};
useEffect(() => {
FetchDetails()
})

Choose a reason for hiding this comment

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

How come you did FetchDetails as a function and not put it straight in UseEffect?

/* This return shows the img, titles, rdate & the id on our HTML page */
return (
<section className="startPage">
{loading ? (<Popcorn />) : (

Choose a reason for hiding this comment

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

Love the loader!

@@ -0,0 +1,13 @@
import React from 'react'

const Popcorn = () => (

Choose a reason for hiding this comment

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

Try to be consistent with the naming, so that the main function and file are named the same

.header-container {
padding: 10px;
background-color: #4c0000;
cursor: url("data:image/svg+xml;utf8,<svg xmlns='http://www.w3.org/2000/svg' width='40' height='48' viewport='0 0 100 100' style='fill:black;font-size:24px;'><text y='50%'>🍿</text></svg>")

Choose a reason for hiding this comment

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

So fun with the cursor! A great idea!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants