@Yago14Submitted 4 months ago
P
vcollins1
@vcollins1All comments
- @vcollins1Posted 2 months ago
solution matches the design
accessibility can be improved by adding link tags
<a href'="#"><strong> Jules Wyvern</strong></a>
0 - @ronrkSubmitted almost 2 years ago@vcollins1Posted 2 months ago
-HTML code looks good but more semantic tags could be use to give meaning. -To improve accessibility you could add tab functionality to the mark selection section and the game board.
- When playing the game I notice that your reset button doesn't bring up the dialog.
0 - @semperprimumSubmitted over 1 year ago
Heyo, everyone!
I have some questions and I would greatly appreciate your feedback!
- I'm unsure about a calculator component having an absolute positioning on desktops. Because a user can switch units to imperial and the whole layout would shift. Absolute positioning prevents that.
- Cards in the "Limitations of BMI" sections also have absolute positioning. I really tried to use grid, but i felt like absolute positioning would be easier and less frustrating to use..
- The curved lines. I'm not really sure if that's how you use SVGs for the background in React (
background-imagein the CSS file doesn't work). SeeApp.jsxand/components/Description.jsxto see what I mean /components/Calculator.jsxmight be too long? I think i could've handled input change inside of the Input component, same with the radio buttons?
- @khaizterSubmitted over 2 years ago
I'm having a hard time implementing the image slider and light box thing I wonder how others did it.
Leave some feedback thanks :)
@vcollins1Posted 3 months agoGood solution, you could use aria labels to improve accessibility.
0 - @KrishnaPoddar1Submitted 7 months agoWhat challenges did you encounter, and how did you overcome them?
Using a hamburger nav-bar and changing the view in desktop view
What specific areas of your project would you like help with?When I change the view in the desktop view the nav-menu is not aligning properly and I think that the view in desktop for the nav-bar is not that good.
Any other suggestions would be welcomed!!
- @KrishnaPoddar1Submitted 5 months agoWhat specific areas of your project would you like help with?
There is quiet a few issues I have a problem finding solution to.
- The Form is not being centered vertically. Even after min-height of 100vh is provided.
- The Radio box when clicked doesn't change its color. The onclick function's have also been mentioned properly and its been called but the console log when it is clicked doesn't work instead I get a reference error. Similarly for handlebox2 as well.
ReferenceError: handlebox1 is not defined
Any Suggestions/tips would be helpful
@vcollins1Posted 4 months agoTo increase the clickable area of your radio boxes you can do something like making the width of the label 100%
.box { ...... display: flex; align-items: center; } .box label { ...... display: block; width: 100%; }0 - @kaoutar-ouadihSubmitted 5 months agoWhat are you most proud of, and what would you do differently next time?
I'm proud of completing this challenge!
What challenges did you encounter, and how did you overcome them?.
What specific areas of your project would you like help with?Anything that can help me improve.
@vcollins1Posted 4 months agoGood solution!! To improve accessibility you could consider adding aria-expanded attributes to your buttons.
<button aria-expanded="false"> .... </button>You can use javascript to toggle aria-expanded= false/true when the button is click.
0 - @kaoutar-ouadihSubmitted 5 months agoWhat are you most proud of, and what would you do differently next time?
I'm proud of completing this challenge!
What challenges did you encounter, and how did you overcome them?.
What specific areas of your project would you like help with?Anything that can help me improve.
@vcollins1Posted 4 months agoYou could make selecting a rating more accessible by adding a role to your list of buttons, and adding a role of radio to each button. Also, an aria-label can be added to the ul so that a screen reader will announce what the group is for.
<ul role="radiogroup" aria-label="rating 1 to 5"> <li><button role="radio">1</button> .... </ul>0 - @bartoszdudziak-devSubmitted 6 months agoWhat are you most proud of, and what would you do differently next time?
- My solution looks quite good and works well even for more than four answers per question
- I am proud of the theme toggle 😎
- I would definitely build the code structure in better way. I split it in functions but it still can be messy for other people
- I think I will refactor it using classes someday 😅
- Also I'm going to add more animations, celebration effect and upgrade keyboard control
- I had to use css custom variables with my scss to make theme toggle works
- I found out that I can't use innerHTML for answers because they contain HTML code 😅
- It was quite easy to make it working but I'm still weak in structure and DRY
I did not use any frameworks and libraries. I think I am getting ready to start learning it. What should I start with?
All comment will be helpful! Please leave your opinion 🫡
@vcollins1Posted 5 months agoGreat job with this solution!! I liked your use of transition animation between questions, will try to use that in the future.
0 - @sasanqcSubmitted over 1 year ago
Hi, every body. this is another solution from me styled with scss. I hope to get some useful comments from dear mentors especially for javascript
@vcollins1Posted 5 months agoTo improve your solution you can factor in the password length when determining the strength.
0 - @JohnMwendwaSubmitted 7 months ago
- @leqsarSubmitted 8 months ago@vcollins1Posted 7 months ago
Good job, but can still use a little work like updating the cards dynamically with selected (daily, weekly, monthly) values.
0
