@AnDevMSubmitted 3 months ago
P
Yulia
@YuliaLantzbergAll comments
- P@YuliaLantzbergPosted 3 months ago
Hi. It's working great. But I can't review the code as it's not formatted. Yes, I think it's a matter of personal (or company's preferences) whether to reset the form or not after sending it. When you reset it, it makes it clearer to the user that something happened when he pushed the button. On the other hand, if the user forgot to say something, or wants to give additional info, he will have to fill the form again. So both approaches have the right to exist. Another thing I've noticed, everything is working perfectly for keyboard navigation, except the concern checkbox. I couldn't find the way to check it without using mouse
Marked as helpful0 - @ZhxixinSubmitted 4 months agoP@YuliaLantzbergPosted 3 months ago
Hi. It's looking great and indeed perfect pixel. Only thing is, I felt it's extremely hard to read code with naming that is not related to the actual page. I mean, shopping cart, product_list, product_page. It really makes it hard to understand wich element on the page is related to the actual code and so it would be hard to maintain or for example to get new members of the team introduced to the code in the real world. In this particular case, if talking react, I'd rather break it into many small components related to the UI. Like ability_score, ability_name, total_score, button. And then to compose them into cards and lists and then to compose them into a page. I mean, like a general idea of course. I hope it makes sense. But from a user perspective, the result is looking really great.
0 - @dar-juSubmitted 3 months agoWhat challenges did you encounter, and how did you overcome them?
This task is clearly not a newbie. For it to work and look nice, you need advanced knowledge of CSS.
Had to work with indents in CSS so that the accordion could expand the number of blocks without rewriting styles.
What specific areas of your project would you like help with?- Due to the fact that a limited height of the text block is used, there are restrictions on the text size. If there is more text, then you need to manually change the height. If make an automatic height, then the opening and closing of the block will not be smooth.
Also, because of the height of the block, there is a twitch when collapsing.
- It was not possible for me to make it accessible from the keyboard without JS (
P@YuliaLantzbergPosted 3 months agoHi. Wow. It's looking like a perfect pixel. Great work. But unfortunately, it is not responsive. It's better to use rems instead of px. And if it's not enough than also media queries. About the second question. You can use tabindex="0" in your HTML code. It will not be the perfect handle of the keyboard. But still, it will enable you to navigate with a keyboard.
0 - @paocomalho1Submitted about 3 years agoP@YuliaLantzbergPosted 3 months ago
Hi Erick. Your page is working perfectly and everything looking great. Just maybe a bit higher, so scroll is not comfortable for this page I think. It's hard to read code as it's a mix of English and another language. The thing that I could notice is, it seems you have a repeating code in mundacaImg and ativo functions. So, you can distinguish it into a separate function. Also, it seems like that will be a better use of switch rather than if else for better readability (but it's rather personal preference). And even better to use the loop, so it can to shorten the code pretty much. Hope it makes sense.
0 - @GodinhoweversonSubmitted 4 months agoWhat challenges did you encounter, and how did you overcome them?
I encountered a challenge with CSS Grid. I'm not used to Grid, but I read some articles on https://css-tricks.com/ that helped me overcome this challenge.
P@YuliaLantzbergPosted 4 months agoHi. Very good solution, way better than mine. I can hardly give helpful feedback here. The only thing is, maybe to wrap the list items in nav into a tag as if it would be a real site, then they'd be hyperlinks. There is also no hover effect on those links. Other than that, code looks so clean and works perfectly.
Marked as helpful0 - @mohamed-fmSubmitted 4 months agoP@YuliaLantzbergPosted 4 months ago
Hi. Good job. But there are a few bugs. First of all the form allows to submit even tho not all required fields are filled. Only name and email are enough to send the form. Also, when navigating with keyboard the radio buttons, checkbox and button are skipped or not skipped but there is no any visual sign that they are focused. Also there is no need to get. But CSS is perfect and there is semantic HTML 👍 Hope it's helpful
Marked as helpful0 - @AnDevMSubmitted 4 months agoP@YuliaLantzbergPosted 4 months ago
Hi. Great work. Everything is working fine. There is a keyboard navigation. Code is clean and simple. The only thing that I could point about, it's no main tag, that is not good for accessibility. Also, and I should say I same the same mistake when was doing this project a few months ago, it's using divs. Rethinking it now, it seems like a good use case for ul and li.
0 - @LesSylSubmitted 5 months agoWhat are you most proud of, and what would you do differently next time?
In this solution, I have added a security feature that does not allow an opinion to be sent without marking a rating.
What specific areas of your project would you like help with?During the creation of the project, I learned many new things that significantly expanded my programming skills. First and foremost, I understood how to efficiently manipulate DOM elements using JavaScript, such as dynamically adding and removing classes or checking the focus state of elements. I also learned how to use CSS animations to create smooth visual effects, like moving elements at a slow pace. Working with event listeners, particularly focus and blur, helped me better manage user interactions. Additionally, I gained practical experience in code optimization, which makes it easier to scale and keep the code readable.
P@YuliaLantzbergPosted 4 months agoHi. It is working well. And also code is written well. You can to make your sass code even better by using mixins. Like for flex configurations that you use several times in the code. It will make it even more maintainable. Otherwise, everything is perfect
0 - @GodinhoweversonSubmitted 5 months agoWhat are you most proud of, and what would you do differently next time?
I am proud of the final result, but in the next project, I will create a folder for each page and a JavaScript file for better organization of the code and improved readability.
What challenges did you encounter, and how did you overcome them?I encountered a challenge using the same JavaScript file for all pages, but for the next project, I will use iframes in the HTML and separate the code into folders for each page.
P@YuliaLantzbergPosted 4 months agoHi. It's working good and responsive. Just a small visual bug. When it shows the correct or incorrect answer, the text is moving to the center. It is because you use justify-content space between on the three elements of the option. You can inject the icon in span inside the p tag, to group it only with the text of the answer and then space between them will move only the icon and text will stay on its place. Or you can use the translateX on the icon. But it is not perfect as you don't really know the distance you want to move it.
0 - @justine1607Submitted 4 months agoP@YuliaLantzbergPosted 4 months ago
Hi. The app works perfectly and desktop screen looks exactly like in design. But it is not responsive. Built only for desktop.
Also, as it is written in react, I think it would be reasonable to break it to such components like Button, List, List_item, Icon. All those reusable and would make the app better for understanding and maintaining.
Hope, it makes sense. :-)
0 - P@KumananEswaranSubmitted 4 months agoWhat are you most proud of, and what would you do differently next time?
What I’m Most Proud Of:
- Dynamic Question Handling: The showQuestion() function effectively updates the displayed question and options based on the current question index.
- Answer Validation: The logic in submitButton.addEventListener('click', ...) for checking the selected answer and providing visual feedback (correct/incorrect icons)
What I’d Do Differently Next Time:
- Improve Event Delegation
- Improve mobile responsiveness
-
Handling Dynamic Data for Questions: I structured the question data into separate objects for each subject (stored in questionSets) and used event listeners on subject buttons to trigger the startQuiz() function, dynamically updating the questions and category-related content based on the selected subject.
-
Preventing User from Skipping Questions: I added validation in the submitButton click handler to check if an answer was selected. If no option was selected, a warning message was shown, ensuring that the user had to make a selection before moving forward.
Handling Text Overflow in Buttons. Some answer buttons have long text, especially on mobile.
P@YuliaLantzbergPosted 4 months agoHi, You js is working great. There are a few problems with CSS though. First of all, you use primarily px, and it's extremely unresponsive. Use rem instead. This site will help you to convert px to rem and vise versa.
About buttons. Remove text-wrap: no-wrap. It actually allows text of the button to go out of its boundaries. Also decrease the right padding (padding: 14px 180px 14px 14px;) 180 is really much there, so it will not be suppressed in the center. In general, you should have been receive the figma files. There you get all the measurements. Just you need to translate them into rems.
Also, you are provided with background images for different screen sizes but I saw in code you use only those for desktop. But too big images stretch the size of the body, so in mobile view the screen is not fixed and moved from side to side.
Hope it's helpful :-)
Marked as helpful0 - P@Y39WebDeveloperSubmitted 4 months agoP@YuliaLantzbergPosted 4 months ago
Hi. Your project is built well. I liked the naming and how js code was written. It's easy to read, the flow is organized simple, and clear. However, I think, the semantics of HTML can be improved. There is no main tag. You can use it instead of the root div. Also you use section as an entire container and then use divs. I think, it would be more reasonable to use a few sections for start, quiz, answers and score as there are separate different groups (pages). Anyway, main is strongly considered as the best practice. And about sections it's only in the scope of personal opinion. Also, there is a small bug. Your progress bar is empty when you enter the first question in the first cycle of the quiz. And start to expand only from the second question. But when you enter the first question after answering other topics, then the progress bar is full on the first question (like it's on 100%) I'd suggest to use the native progress tag of HTML. It'd be more right semantically but also very easy to manipulate. You just give the number of questions as max and change value as a number of the current question. And it will work perfectly. Hope it is helpful. And again, I really liked the simplicity of js code. :-)
0 - P@ashmaddenwebmelbourneSubmitted 8 months agoWhat challenges did you encounter, and how did you overcome them?
Managing state and trying to figure out what parts should be separate components. Also deciding on whether to keep types in the React file or put them into a separate folder.
What specific areas of your project would you like help with?Organising code and separating functions to have smaller component file sizes and more organisation.
P@YuliaLantzbergPosted 4 months agoHi Ash. Good job. The quiz is working well, without bugs. However, there can be some improvements in code. I think, React is a great choice for this project. You separated the pages and that's great. But also when we look at the design, there are some repeating components. Such as List and List Item (you can use them in the welcome page and question page), Button(question page, submit page, and final page), Icon (in every list item in the menu, question, header title, and in final page title), Title(it looks the same in the header and in the final page). Also for managing state, I think there the context API and state hook would be in place here.
Also, I think there is no keyboard navigation, or I didn't figure out how it works. In CSS, the moon and sun icons in dark mode stay dark and so hardly visible. Hope it makes sense. :-)
0 - P@YuliaLantzbergSubmitted 4 months agoWhat are you most proud of, and what would you do differently next time?
Hey all. Here is my solution for "Newsletter Signup" challenge:
Scored 95% for mobile and 99% for desktop on Google Pagespeed Insights! 🚀
Solution with 100% W3C validation accuracy 🌟
There is an email validation. When you click on the dismiss button get back to the signup page and the input field is cleaned.
🛠️ Built with:
HTML ✨ CSS 🎨 JavaScript 🧾
Used BEM naming style for CSS. Fully responsive design crafted with a mobile-first approach 📲
What challenges did you encounter, and how did you overcome them?Css is still a struggle for me 🤯 and work with javascript is a joy 💥 Anyway it's getting smoother.
What specific areas of your project would you like help with?Any feedback would be appreciated. I got a weird situation and can't understand it. Though the added active class included display:"flex", it was crossed in browser (not working). I had to toggle directly from js flex and non for display. So maybe anyone knows what could be the reason that display stayed in none mode, even tho active class was added
P@YuliaLantzbergPosted 4 months agoHi Andrey. I really appreciate your detailed review and efforts you took to go through my code in attention. It's very helpful. You are right about regular expression. I've added and saved the sample you gave. Thank you so much. Indeed, the entire content was moving with hover on the button. I wanted actually that only button will move. I didn't take in attention that margin can trigger the entire scope. Now I replaced margins with translateY and it makes work perfectly, moving the button without disturbing other elements. About the flex, unfortunately, it still doesn't work for me if to remove the mentioned lines. I am talking about the mobile view only. On other views it works perfectly if to remove them.
And again, thank you so much for your feedback. Really appreciate
0 - P@catreedleSubmitted 8 months agoWhat are you most proud of, and what would you do differently next time?
I am proud of creating a working password generator, it still needs improvement in determining the password strength.
What challenges did you encounter, and how did you overcome them?The biggest challenge was customizing the range slider. I found a helpful article https://blog.logrocket.com/creating-custom-css-range-slider-javascript-upgrades/
What specific areas of your project would you like help with?Any feedback in general, especially in Javascript.
P@YuliaLantzbergPosted 4 months agoHi Purnama. Nice work. However, there are a few things that I've noticed.
- There is no main in your HTML. The HTML doc must have the main tag. Also, you use span as an independent tag. Those are important points for accessibility.
- The bullet of the slider remains in the active state even when I leave it and not hover until I click somewhere else. It gives the feeling that something is wrong or frozen.
- In checkPasswordStrength() you have first if else statements and then switch. The switch is unnecessary. In fact it makes code less maintainbale and vulnerable for further bugs. You can remove "hidden" class in the same if else statements.
- There are separate getRandom functions for each case and each of them has almost the same code. Maybe it's better to write the generic getRandom function that receives the min and max values.
I hope that makes sense :-)
Marked as helpful0 - @SergioCasCebSubmitted 4 months agoWhat are you most proud of, and what would you do differently next time?
Fun little challenge. I would probably improve the architecture of the app to optimize the JS functionality.
What challenges did you encounter, and how did you overcome them?Nothing in specific.
What specific areas of your project would you like help with?Nothing in specific, but any feedback is always welcome. :D
P@YuliaLantzbergPosted 4 months agoHi, Sergio. Great work. I liked that you used the ratio buttons for the tip buttons. And the code is well structured. The only thing that I see for improving it's the error messages. When I'm entering the negative value, still I get message about zero. Also you disable the reset option until all inputs entered correctly. I think it is not really convenient for a user. Anyway, those are more UX things than coding. Code itself is really good
Marked as helpful0 - @SergioCasCebSubmitted 4 months agoWhat are you most proud of, and what would you do differently next time?
Fun little challenge. I would probably improve the architecture of the app to optimize the JS functionality.
What challenges did you encounter, and how did you overcome them?Nothing in specific.
What specific areas of your project would you like help with?Nothing in specific, but any feedback is always welcome. :D
P@YuliaLantzbergPosted 4 months agoHi, Sergio. Great work. I liked that you used the ratio buttons for the tip buttons. And the code is well structured. The only thing that I see for improving it's the error messages. When I'm entering the negative value, still I get message about zero. Also you disable the reset option until all inputs entered correctly. I think it is not really convenient for a user. Anyway, those are more UX things than coding. Code itself is really good
0 - P@marcus-hillSubmitted 4 months agoWhat are you most proud of, and what would you do differently next time?
My use of JavaScript to populate the DOM on page load (with all the data) and also run the same function when the timeframe is changed (monthly/weekly/daily). I think my HTML has been structured well and I have made it as accessible as possible.
What specific areas of your project would you like help with?Please provide some feedback on my JavaScript code.
P@YuliaLantzbergPosted 4 months agoHi. Your code is working well but there is place for some optimization.
You reach DOM each time when the button is clicked
document.querySelectorAll('.time-frame'). But you anyway need the same data to reach buttons. So, it's better to take it out of the populateDom function and to put it in the top level code. Then to loop over it and to add listeners for each item. So, you will need to reach DOM only once instead of each click.0
