Design comparison
Solution retrospective
-It was a challenging project to build, especially making responsive design was took some time but helped me to sharpen my skills.
-I think i should have create more re-usable scss components for repeative structures. And i purposely did not change the font-family and added some differences to my UI.
-If you have any feedback about design and the code, feel free to comment or mail to me.
Community feedback
- @JagholinPosted over 1 year ago
Good work
some things that I noticed in the UI:
-
on the first page, if you dont enter anything and just try to press "next step" button, there are no error messages saying that some fields are required.
-
phone number field doesnt like spaces, even though the placeholder message suggests otherwise.
-
on the second page, the text in panels "arcade" "advanced" "pro" can overflow on some viewport widths(just large enough to still be in the desktop layout)
-
in some circumstances(not sure yet when) the "monthly <-> yearly" switch can break and get stuck in one position.
-
the remaining 2 pages seem to be fine, didnt break :)
-
You dont use any libraries to help with managing state or form validation? impressive. I would recommend Formik or react-hook-form
-
regexes: phone numbers can have 0 after +. Also interesting that you included so many characters in the name regex, but left out "ä". (I dont think you even need name regex, what yre you trying to check?)
-
I see you are using viewport units. Nice! There is this video by Kevin Powell about them that goes into some problems that they might cause sometimes and how to fix them.
-
speaking of CSS, I would add layout breakpoint
@media screen and (max-width: 768px)
to the variables, because its used everywhere, and is something that needs to be tweaked from time to time -
didnt even try to make nice circles around step numbers in the sidebar? The easiest way is to just give them definite height and width, like
height: 4rem; width: 4rem;
-
you use
let
s too much, try to useconst
keyword wherever possible. -
useEffect
on line 91 in file app_context.jsx can be rewritten to grab data from data.jsx about available plans and options and use it in calculation instead of hardcoding a bunch of nested ifs. Also dont put dependent variables liketotalPrice
into your context, but calculate them on the spot when you need it(and you can useuseMemo
hook to memoize the value). The principle here is 'make invalid states unrepresentable', in this case, we dont want totalPrice to become desynced from the rest of the state by some mistake. - <nav> element is supposed to contain links, but yours doesnt. Also <button> in the navbar is misleading, if it doesnt do anything.
-
in vitejs, you can import an asset like an svg directly into your js file. You dont need to inline their content manually like you do in data.jsx.
-
The "payment plan" switch breaks, because you dont initialize it with correct state from your context in SelectPlan.jsx. This should be something like
<IOSSwitch onClick={changePaymentPlan} value={payment_plan} />
. -
You have some accessibility problems(images without alt text, bad tab ordering, and you are using heading levels for styling ). I suggest running accessibility tool in DevTools
And thats all I could find. Dont be discouraged by all this stuff, your code quality is one of the best I ve seen on the frontendmentor.io so far and is easy to read and understand.
Marked as helpful0@yigithancolakPosted over 1 year agoWow go easy on me i just doing this for 4 month :)) First of all thank you for looking at my code very detailed and being so kind to not discourage me. It was a very good vision. I know my app has to be more UX friendly i will improve that later on. About the switch break.. I dont now why this happening but i will try to give it a value but the value i was changing is a string it only can be 'monthly' or 'yearly'. I dont now how convert it to true or false but we will see. And also big thank you for the svg thing it was the first time i was dealing with svg's. Can you give me a example of importing it ? @Jagholin
0@JagholinPosted over 1 year ago@yigithancolak You can convert "yearly" or "monthly" value to a boolean by comparing it to another string. Like
payment_plan === 'yearly'
for example.With vitejs, you can import image files in your sources, like for example
import myIcon from '../assets/icon.svg'
. In this case,myIcon
will contain an URL of the image by default, which you can then assign for example to a source attribute of an<img>
:<img src={myIcon} alt="" />
. Dont forget to addalt
attributes, the empty string here means that the image doesnt have any special meaning and is just a cosmetic element.Marked as helpful0@yigithancolakPosted over 1 year agoAlso do you think the folder structure is good ?
@Jagholin
0@JagholinPosted over 1 year ago@yigithancolak its fine. Personally I also like to separate components into "pages" and "controls"(things like buttons, input boxes etc), but I dont think its a requirement
Marked as helpful0@yigithancolakPosted over 1 year agoThank you for the feedback have a good day @Jagholin
0 -
- @MaeWolffPosted over 1 year ago
Hi ! Congrats on posting your challenge :)
You can improve the accessibility of the
add ons
step by using the label tag on your card to select the checkbox by clicking anywhere on the card.You can check this example: https://flowbite.com/docs/forms/checkbox/#advanced-layout
0
Please log in to post a comment
Log in with GitHubJoin our Discord community
Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!
Join our Discord