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

Survey - v6 #438

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

Survey - v6 #438

wants to merge 14 commits into from

Conversation

JohLeo
Copy link

@JohLeo JohLeo commented Mar 19, 2023

import { Who } from './components/Who'
import { Summary } from './components/Summary'
import { Breathe } from './components/Breathe'
import hug from './img/hug.svg';

Choose a reason for hiding this comment

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

Didn't know you could import images as well as components. Interesting!

</div>
</>
)}
{step === 8 && (

Choose a reason for hiding this comment

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

I loved this part! It was like a surprise bonus at the end.

export const Activity = ({ activity, setActivity, step }) => {
return (
<>
<div className="content-container">

Choose a reason for hiding this comment

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

I notice some of the classnames aren't in camel case. Not a biggie, but maybe for future reference?

<div>
<p className="p-step">Current step: {step} / 7</p>
</div>
<h4> Where do you feel the most at ease?</h4>
Copy link

@SofiaGerdmar SofiaGerdmar Mar 22, 2023

Choose a reason for hiding this comment

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

Really nice with the images above the choices. Elevates the whole thing! I would recommend making the files a bit smaller though. As it is now it takes a while for the images to load, which takes away a bit from the experience.

height="170"
fill="none"
strokeDasharray="20 20" />
<circle cx="10" cy="10" r="8" fill={colour} className="blurryCircle">

Choose a reason for hiding this comment

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

This was really the icing on the cake! Managing to implement a moving image that is also relevant to the excercise, amazing!

<>
<div className="content-container">
<div>
<p className="p-step">Current step: {step} / 7</p>

Choose a reason for hiding this comment

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

Nice job displaying current step!

<p className="p-step">Complete 7 / 7</p>
<h4 className="sumh4">So...</h4>
<p>
{name}, picture yourself by the {place} where you are about to start your {activity}. <br />

Choose a reason for hiding this comment

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

Great way of showing the results. They look so much better in a story than in a list.

cursor: grabbing;
}

@media (max-width: 668px) {

Choose a reason for hiding this comment

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

If you reduce the width a bit, or add margin: 0 here the container will fit iPhone5/SE too. Right now there is a horizontal scroll on this step on the smallest phone.

@@ -0,0 +1,48 @@
import React from 'react';

export const Temp = ({ temp, setTemp, step }) => {

Choose a reason for hiding this comment

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

Nice feature!

onChange={handleWhoChange} />
</div>
);
}

Choose a reason for hiding this comment

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

Nice, clean code all throughout this project. Apart from a few minor issues, it works super well! Good job!

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