@JunasVee
@HussinSoli
Submitted
Feedback is much appreciated , still learning react had some issues with the context I made the state keeps making infinite loops. I made the slider and the navbar into to components one for mobile and the other for desktop, so is this a good way or there is a a better one?
@JunasVee
Posted
Hi Hussin!
Your page is responsive and it looks good to me, I can't really mention all of the issues since you said you're still learning reactjs which I think it's common to have difficulties functionalizing particular things. But here are the issues that are such a "big deal" in my personal opinion:
The cart, it's one of the main concepts in this challenge. In Ecommerce Product design, having a cart like this makes the customers know what they have been putting on. I saw that you have set the function for it but it doesn't seem working on my device, please let me know if it actually works.
Increment and Decrement, I tried to decrease the number when it's 0. it's good that the output doesn't show a negative number, but if you click it 3 times after 0, in order to put it back to 0 it also takes 3 increment clicks, so behind the scene, it is still a negative value without showing it to the output. So instead of creating a string for negative values, when counter <= 0 set its value to 0 too.
I hope my feedback is helpful.
Marked as helpful
@KohseyPower
Submitted
Feedback welcome ! ^^
This is my first submition. I'm trying to learn and practice HTML and CSS. There are probably bad practises, notice me !
@JunasVee
Posted
Hi Kohsey!
I've checked your preview site and its codes in your repository. It looks awesome to me, simple codes only a few lines and the CSS is short as well but everything works fine. Additionally, it's responsive to all screen sizes. I would say that your efficiency is at another level!
Perhaps the things that you might want to fix are accessibility issues in the report but those issues aren't extremely important. Overall, absolutely well done.
Marked as helpful
@Kratos012
Submitted
Well, I'll appreciate some feedback from you guys. Thanks I've still got a ways to go.
@JunasVee
Posted
Hi Kratos012!
Congrats on finishing this challenge, I gotta say that your solution looks clean and almost perfect. I'll kick this off by answering your inquiries:
I know that this challenge tells you to make the solution looks as close as possible to the example design and probably it includes the font-size, but when it comes to a real-life situation, what actually matters are its readability and its suitability with the web design. This might be a bit off the box but trust me, it would be more useful once you achieve something in this field.
px unit is good when a page is made for a single specific viewport and defines the height or width of an element. Most developers use Relative Units such as rem and em for font-size, vh for page's height, and vw for page's width. I personally use % for margin and padding to make it responsive. To fit this particular image, I guess it'd be better to use the img tag instead, I've tried it, and actually worked, additionally, you can make the container's width wider.
My Feedback:
The rest is pretty much well done. I hope my feedback is helpful.
Marked as helpful
@afaiz-space
Submitted
my second challenge on front-end mentor.
@JunasVee
Posted
Hi Faizan!
Good to know that you've finished your 2nd challenge on this platform, your solution looks good, however, I've found several issues that you might want to fix. Here are the issues:
Flexbox, the Main Container should always be both vertically and horizontally centered but in your case, it's not vertically centered even though you've added align-items: center; on the main tag. The problem is, you haven't set the height of the container, therefore it makes the flexbox confused because by default it follows the child element's tallest height. To fix this, add
min-height: 100vh; for the main tag and your issue will directly be fixed.
Font-Family I suggest you read the style-guide.md file, it says that the font-family should be Lexend Deca but it's not implemented in your solution. To fix this, import the font from the google font website and set it as the font-family for every element.
Styling/CSS, We don't need to add a type attribute to a style tag because HTML already knows what it is, removing it would be better because it has no value and is making the code looks filthy. Additionally, it gives you an HTML issue report for this challenge.
Main tag, instead of setting a div as the main tag, you can directly use the main tag itself. As a matter of fact, it provides a better Search Engine Optimization(SEO).
There are actually a few more issues including the texts but I guess it would take too long to type. The point is to keep learning from your mistakes because it's better than not making anything.
I hope my feedback is helpful.
Marked as helpful
@dawidPoznanski
Submitted
Feel free to give me a feedback 😁.
@JunasVee
Posted
Hi Dawid!
It is great to know that you finished this challenge, your page looks nice and interesting. However, there will always be room and time for improvements. Here are the issues that I found in your solution:
On my screen, the text is big enough and clear, great. But when I tried to open it on bigger screen size, it looks so tiny. Perhaps you can make a media query for a viewport larger than 1800x1000 and increase the content's font-size until it is readable on large screens.
I saw that you haven't given your background the properties that it needs. It's duplicating or in the Web Development, we call it background repeating.The size is smaller than most desktop screen sizes as well. In the order to fix that, you can put background-repeat: no-repeat; background-size: cover; background-position: center; on the body tag in CSS.
I'm not sure why, but I think there should only be 1 footer tag in each document. To fix this, either wrap the social media components in a div instead of a footer tag or delete the attribution with its footer tag. It makes better accessibility.
This is the biggest issue, you didn't add the mobile media query. I suggest you make one because it is really important since everyone has a mobile phone and tends to do anything from their phones.
The point of these issues is that your page isn't responsive. But it's all back to you whether you want to improve or not, I'm not forcing you, I'm just here to give you the feedback that I think I better give. Keep going 👍
Marked as helpful
@DawidMTX
Submitted
@JunasVee
Posted
Hi DawidMTX!
I saw that there's no orange box image file in your folder, so I assume that the problem is that the wanted file is missing.
Your solution works as it should, great. But it seems like it has some issues with the UI/UX or the styling. Here are the things that you might want to fix:
box-shadow is too big that it makes the violet background looks really dark near the card.font-size a bit for both headings and p tags.align-items: center; to the main tag in the CSS so that it'll vertically centers the components in any screen size.You provide what the challenge wants you to provide regardless of how the output looks but improving it would be better, Great Job and keep going.
Marked as helpful
@remyboire
Submitted
Altought it is not perfect, it has been a good way to learn a bit of algorithmic thinking. I tried to make the computer play smart and block the player's moves. CPU plays randomly, but if its opponent can win, it will block it, otherwise, if CPU can win, it will play to win. This is basic, but seems to get people into enough trouble.
Any feedback is welcome !
@JunasVee
Posted
Hi Remy!
I would say that this is fantastic! I tried to play against the comp and I couldn't win even a single round. The Front-End looks absolutely interesting. I visited the JS file and there are tons of lets but still looks clean and the logic/algorithm is out of my mind, all buttons and their functions work properly. I guess your intention to get people into enough trouble has been fulfilled as well😂👍
Additionally, no accessibility or HTML issue. Definitely 10 out of 10, you made me learn a lot from your solution.
@LiliYao2022
Submitted
Hi there, This is my first coding challenge, actually I am a bit confused about how to set mobile size or desktop size for my design. Also, I learned about " display: flex, oh, it is so powerful, thank you!
@JunasVee
Posted
Hi Lili!
It seems like something is wrong when you uploaded your files to Github. The preview link only shows the README.md file's content. What I suggest you do is:
After you implement this solution, I'm 99.9% certain that your website will work as expected.
Marked as helpful
@offedward
Submitted
@JunasVee
Posted
Hi! It looks like your codes are the same as mine. They way the output works, the class and ID names, as well as the javascript codes. In fact, the JS code is 100% the same as mine! considering that I submitted the solution first, I assume you copied my code and this is against my owning rights! I suggest you delete or change this solution before I claim this as a copyright strike!
