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

Alexander : Week-6, Survey #445

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

Conversation

Alexander-Gabor
Copy link

Copy link

@dannebrob dannebrob left a comment

Choose a reason for hiding this comment

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

What a well-made survey, I'm so impressed by your design! It's fun, works well with your theme and is nice to use. Great job Alexander!
You have made a great effort in this weeks project, and I have only added smaller comments on a few things in your code. No big deals.

Comment on lines +28 to +40
{products.map((product) => (
<label htmlFor="product" key={product} className="favorite-product">
<input
type="radio"
name="favoriteProduct"
className="product-button"
value={product}
onChange={handleProductChange}
onChecked={favProduct === product}
required />
{product}
</label>
))}

Choose a reason for hiding this comment

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

Nice use of map()

const handleStepIncrease = () => {
if (name.length > 0) {
setStep(step + 1);
setPercent(percent + 20);

Choose a reason for hiding this comment

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

You have a lovely progress bar; I like it a lot!
Now you have an easy way to calculate how many percentages the progress bar should increase, but imagine that you don't want to do the math yourself; then you could use something like this:

const correctPercent = (step * 100) / 6;
setPercent(Number(correctPercent).toFixed(1));

The / 6 is how many steps you have in your form.

Comment on lines +47 to +53
{/* <input
type="text"
value={food}
ref={inputRef}
onKeyDown={handleKeyDown}
onChange={handleFoodChange} />
{console.log(food)} */}

Choose a reason for hiding this comment

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

No need to reference past code if you have another working solution

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

const Landing = (props) => {

Choose a reason for hiding this comment

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

Unused component? I might need to be corrected here.

import ThankYouImage from '../images/thankyou.png'
import Purchases from './Purchases';

const allRange = ['It sucks', 'It is aiiight ', 'It is okay', 'it works niiiicely', 'Superb /chefs kiss']

Choose a reason for hiding this comment

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

I thought this was so great, very funny! Fantastic implementation as well!


/* Tablet MQ */

@media (min-width: 768px) and (max-width: 991px) {

Choose a reason for hiding this comment

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

The app has some design differences on smaller devices (<768px). Making the form's background on smaller screens the same as on tablet/desktop could make it look more aligned with the desired experience. But this is just based on my preference for design.

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