A.Tech Services• 100
@A-Tech-Services
Submitted
Pls i need someones to help me go through my project and the code i am very sure i need a lot of corrections and guide.
pls do help drop review and how i can improve
Remtaine
@remtaine
All comments
- Remtaine• 360
@remtaine
Posted
Did you upload 2 pages? The results page doesn't show up or doesn't work. Either way, I suggest you practice working it as a single component so that it doesn't need to go to another page.
Right now, I can select several numbers at the same time. I suggest tracking the currently selected button so you can inactivate it before activating the clicked one.
Regarding the design, I suggest increasing button size and lessening the button spacing.
All in all a good try! Make the appropriate changes and it'll be great!
Marked as helpful
1 - Daniel• 680
@DanielArzani
Submitted
Remtaine• 360@remtaine
Posted
Good job! The design looks really good. The images line up when I'm at 90% zoom, but at normal zoom and others, they tend not to be aligned. I suggest having a container div that takes up all the space you want, then use object-fit to make sure the image always fills the appropriate space regardless of aspect-ratio. Also, while it's not included in the challenge specs, some transition animations would look great as well.
All in all it looks great though! Great job!
Marked as helpful
1 - Daniel Earp• 100
@Earpz1
Submitted
I'm very new to front end work and I am currently completing an intensive coding bootcamp. This is my first attempt at a Frontend Mentor challenge & only my second attempt at creating a CSS design. I think I've got close to the spec but I'm sure I didn't do it the "best way" so any advice on that would be great.
Remtaine• 360@remtaine
Posted
The design looks pretty good. For the content, it's better if instead of manually centering it through pixel margins, you center it using either auto margins or flex. That way you don't have to calculate every time. Also, the text at the bottom overflows. If you want them all contained in the parent white box, don't set a constant height and just add padding for spaces.
All in all it's good though! Keep it up!
Marked as helpful
0 - saskue7• 170
@saskue7
Submitted
Main difficulties are the script part how you connect all the dots is the main idea here.
Remtaine• 360@remtaine
Posted
Hi saskue7! Great effort on this challenge! Regarding the script, I checked out your JavaScript file and here are some of my comments:
-
Use better variable names and types. For example, your variable "flag" could have a more understandable name like "hasSelectedButton" and would work better as a boolean (ie. true, false) rather than an integer (ie. 1, 0).
-
Also, instead of having a boolean for that, a better solution would be if you just had a variable store the current active button and check if it has one when clicking another rating button or the Submit button. That way:
a. For the first time selecting a rating, change active button and toggle the "showbutton" class on it
b. If changing the rating, toggle class of current active button, change value of active button, then toggle that as well
c. If deselecting the current active rating, toggle class of current active button, change value of active button to null
d. When clicking submit, you can check if active button isn't null. If so, nothing happens. If it does have a value, get the value of the active rating button and output it.
Regarding the design: maybe you changed the font and color on purpose, but for future challenges I suggest you follow what's listed in the style guide. That way, it'll be easier to comment on differences.
Overall it's a nice effort, and I'm sure you'll do even better in future challenges!
Marked as helpful
0 -
- Mohak Muskan• 10
@mohak-muskan
Submitted
I only came up with a desktop preview and not for mobile. Mostly because it lacks responsiveness. Any tip on how can I work on that?
Remtaine• 360@remtaine
Posted
Looks pretty good! Regarding responsiveness, look into either Flexbox or Grid for this. Both can work, though I'd suggest Flexbox since all you'd have to do then is change the flex-direction based on screen-size through media queries. Flexbox is better for lines (a single row or column), Grid is better for work with the x and y-axis at the same time.
On my desktop browser (Firefox), there's also the trouble of the image not filling in the height. It's just in the upper left corner. Proper usage of Flexbox properties like flex-grow, flex-shrink, and flex-basis can fix that.
All in all, a great effort! I'm sure once you apply the changes it'll look even better!
0 - Oluchi Onyeguke• 190
@lulublake
Submitted
I have been struggling to adjust the hover icon cos its an svg file... any idea on how I can go about it???
Remtaine• 360@remtaine
Posted
Looks good! Aside from the mentioned problem with the eye centering, everything seems to be in order. I suggest you look into flexbox, it'll really help with regards to centering. The eye image being an svg shouldn't matter, any image type can be centered like that.
Apart from that, it looks great! Good job!
Marked as helpful
0 - PMarcell Mihály• 750
@mihalymarcell86
Submitted
Remtaine• 360@remtaine
Posted
Looks pretty good! I love the starting animation for the illustration. It looks great on both mobile and desktop. One thing to improve would be that at certain screen sizes for desktop, the illustration at the left side tends to overflow. All in all though, I'd say you did a great job!
0 - Pius Prosper• 10
@Pptechie
Submitted
Remtaine• 360@remtaine
Posted
Right now the site is down or not working since it's showing a 404. I suggest you edit that so people could comment on the live site
0 - MohammedS2000• 30
@MohammedS2000
Submitted
any tips are apprecited
Remtaine• 360@remtaine
Posted
It looks more or less okay at first glance. For things to fix, make the cyan overlay the same size as the image, which could be done by having them in the same div and making them take the full length and width. More spacing would be beneficial for readability. Also make sure to not wrap the rows containing the ETH and days left, as well as the profile image so it'll be similar to the design. I suggest you look at the style guide as well so you know what fonts to use.
Marked as helpful
0 - Ajay• 50
@Away87
Submitted
The texts are actually elevated up a bit. It messes with centering. If you look closely in the rating icons, you can notice the text is not centered correctly. I found that it is being caused due to the font-family. Anyone know the Issue ? Other Feedbacks are appreciated. Thank you in advance
Remtaine• 360@remtaine
Posted
Looks pretty good! The logic for the rating input works as well. Regarding centering text, I'll throw out some ideas you may try. One method would be turning the buttons into a flexbox and then centering it on the y axis. Alternatively, you may also try setting the vertical-align to middle. You could also just add a bit more padding at the top of the text, though that's a less responsive solution.
0 - Juanma Romero• 110
@juanma-romero
Submitted
Remtaine• 360@remtaine
Posted
Looks great! The JavaScript works and reacts to changes in input. I also like the flip animation once you click Submit. As for points for improvement, at the mobile breakpoint (375px) It loses its margins, so I suggest you ensure that the margin doesn't disappear at smaller widths. You can do that by having a more flexible width size. Overall it's pretty good!
Marked as helpful
0 - Donovan Acosta• 70
@donovanacosta20
Submitted
Remtaine• 360@remtaine
Posted
Looks good! The JavaScript works and it reacts to mouse inputs. In my browser, however, the Submit button overflows from its container. It seems you hard-coded the container's height so if it doesn't fit it would overflow, so I suggest having a more fluid design that ensures everything fits. Same with the width, it's hard-coded so at the mobile breakpoint (375px) it doesn't have a border and feels cramped. Add some margins so it looks better for phones. Aside from other slight details like spacing and button sizes, it's pretty good!
0
