@Glenda9031
Submitted
I couldn't get the hide/show JavaScript to work for me. If someone could please offer any suggestions or advice? Happy Coding!
@piushbhandari
@Glenda9031
Submitted
I couldn't get the hide/show JavaScript to work for me. If someone could please offer any suggestions or advice? Happy Coding!
@piushbhandari
Posted
i had a rough look at your JS and styles.
make sure to:
display:none;
to your .accordion-text element to hide it when the accordion is not opened..title__click{display: block;}
this is so that when the class gets added to both elements you'll be showing the .accordion-text element too. then when you click again you can use JS to remove .title__click
BTW:
<label for="section1" class="accordion__label title__click" id="title">
will be <button type="button" class="accordion__label title__click" id="title">
alt=""
since this is for decorative purposes only. i would also set the image to be width: 100%;
so that when you zoom out your layout doesn't look cropped outhope this helps
Marked as helpful
@SebBudynski
Submitted
Hey, It's my first project on Frontendmentor where i used JavaScript. I'm very happy of progres i'm making by doing this exercises. Happy to hear what i can improve in my code.
Regards Sebastian
@piushbhandari
Posted
.questions:last-child{}
to remove the border + bottom margin/padding from the last question<div class="question">
isn't a button element? right now i can't use your component with my keyboard. using semantic elements is always a good thing. there are more accessible things you can do but replacing it with a button element is a good startdisplay:flex;
<div class="title">
is being used as your main title replace the the div with <h1>Marked as helpful
@danielmrz-dev
Submitted
Hello! I'm Daniel and this is my solution for this challenge! 😊
This project took my Javascript
skills to the next level. The design is very simple, but the many different validations were definitely a great challenge. At least for a Javascript Newbie like me. For the first time I had the opportunity to use switch case on a project. My index.js
file is a huge monster 😅! But in the end I managed to add all the required features.
I also added some extra features like a counter up animation for the age result and changed the colors.
If you have any suggestions on how I can improve this project, feel free to leave me a comment!
Feedback welcome 😊
@piushbhandari
Posted
nice work.
some suggestions:
i would make these <h2 class="text-day" style="color: rgb(113, 111, 111); transition: all 0.2s ease 0s;">Day</h2>
into a label and hook it up to the number inputs. like so: <label for="day" class="text-day" style="color: rgb(113, 111, 111); transition: all 0.2s ease 0s;">Day</label>
then add in name/id attributes that have the same value as the for attribute to the input. this way when you click on the label it auto focuses on the input. this is good for accessibility
your .text-day element has an inline style. is there a reason why it's not in a separate stylesheet?
it's best practice + good for accessibility if you add type="button" on your button elements
you can set the alt tag from the icon inside the button to be empty "" as it's useless information for screen reader users. when you have icons, having alt="" is good enough as it's being used as a decorative image
im not sure if there's a tablet design, but when you start to scale the design, the component starts hugging the edges of the screen. i would add side paddings ie. padding-left: 24px; padding-right: 24px;
on the body element for example.
may need a 2nd opinion on this but those years/months/days text can just be regular <p> tags instead of <h2>s
@123Joseph-Ife
Submitted
this helped me practice my form validation
@piushbhandari
Posted
form validation needs to be stronger. i was able to get to the next screen with this: ggggsdff@com (no '.' after the @)
by the way, i wouldn't use cursor: pointer; for the input and instead use the default style on browsers
Marked as helpful
@ddkogit
Submitted
Is this optimal way to do?? toggling class, or if there is other please feel free to recommend.
@piushbhandari
Posted
toggling class to show/hide content via javascript is a good approach and you'll see this as being best practice.
BTW, when i zoom out on your page, your layout gets messed up. i would change your styles on your body element to background-repeat: repeat-x; background-size: contain;
. FYI, i would have the background image as a separate element
i would change <div class="question">
to <button type="button" class="question">
. it's good to use semantic markup + as it stands right now your component is not accessible via keyboard and that's a no no. likely will need to update styles
Marked as helpful
@ObiFaith
Submitted
What did you find difficult while building the project?
Do you have any questions about best practices?
@piushbhandari
Posted
id="email" name="email"
. this is for accessibility. when you click on a label, it should then focus on the input it's associated tolet me know if you need help with the last one. essentially what i did in my markup was attach this onsubmit="return false;"
to my form element which blocks the form from doing its native action
Marked as helpful
Any feedback is welcome but especially help about mobile border-radius I didn't know how to do it
@piushbhandari
Posted
for border-radius, you can set it like this: border-radius: 10px 11px 12px 13px;
top left, top right, bottom right, bottom left in that order
FYI: https://developer.mozilla.org/en-US/docs/Web/CSS/border-radius
so in your code for mobile, you can set the first card to be, for example, border-radius: 5px 5px 0 0;
, border-radius: 0;
for the middle one, then border-radius: 0 0 5px 5px
for the last one
Marked as helpful
@AyouubElb
Submitted
@piushbhandari
Posted
the form accepts my email before i can even finish it. it auto accepts instead of me clicking the submit button
the dismiss button doesn't work. i would assume this would just take you to the first screen
@luismadf
Submitted
Hello,
I decided to go with Vite and React to create this project.
Any feedback will be appreciated, I have some doubts about the open/closed animation in the accordion.
Thanks
@piushbhandari
Posted
you should look into changing your markup to be more semantic. for example <div class="accordion-item__title"><h3>How can I get help if I'm stuck on a challenge?</h3><img src="/assets/icon-plus.svg" alt="toggle view button"></div>
can be converted to <button type="button" class="accordion-item__title"><h3>How can I get help if I'm stuck on a challenge?</h3><img src="/assets/icon-plus.svg" alt="toggle view button"></button>
then adjust your styles.
the reason why is because right now your component is not accessible - it's not reachable for keyboard users. add hover/focus states too
also change your max-height to be 100%. take a look what happens on mobile when there's more text than what you have currently. (it overflows)
@Fluffyboiiiiii
Submitted
Hello, I didn't have many problems while making this. However, if someone could answer this questions I would greatly appreciate it!!
How can I change the Box Shadow of the .card class hovering over its child? In the Active States image, it shows that when you hover over the "HTML & CSS foundations" the box shadow gets slightly bigger.
How can I make the "HTML & CSS Foundations" heading focusable? in the README file it says that all interactive elements should be focusable. However, I have no idea on how to make this, I found some examples on how to do it but they all require JS.
Is the code well written? I have gotten some feedback in the Discord server saying that I should have gotten some previous feedback on the QR code component, but I honestly don't know what's wrong with the code.
That's all for now, have a nice day!
@piushbhandari
Posted
surround your h3 into a link like so: <a href="#"><h3>HTML & CSS Foundations</h3></a>
likely will need to do style updates. no JS required
this way the title is focusable. if this was on a real web page, this should also be taking you to a different page hence the link.
if you want the box shadow to increase on hover/focus, play with the property on dev tools to get what you want. not sure if i would go this route because this would assume that the whole card would be surrounded with an <a></a> element, and that's bad for screen reader users. but i suppose if you just want a hovering effect, you can do something like .card:hover { box-shadow: 17px 17px black;}
then of course add a transition to it to make it smooth on hover
@lucky-lore
Submitted
-------- All Frontend Mentor Challenges --------
Hello there! 👋
I'm a professional React developer working through the challenges from Frontend Mentor. 3 years ago doing the Guru projects landed me a job and I am coming back to finish all the challenges as a hobby. I'm methodically progressing through these challenges, starting with the free versions, and gradually working up to the premium ones.
-------- My Journey 🚀 --------
-------- Repository Overview 📚 --------
The repository is a Lenra Monorepo. Each project in /packages
is hosted on Vercel. The live versions you can find in the README.md
-------- Project Structure --------
Each project is housed in its own folder in the packages
directory and contains all the necessary files. Feel free to explore.
-------- Stack That I will be using for the projects --------
-------- Future Plans 🌟--------
-------- Connect with Me and Support My Journey! 🤝--------
Your support and feedback are crucial to my growth as a developer. Let's connect and embark on this coding journey together!
Thank you for stopping by! 🙏
@piushbhandari
Posted
for that wavy pattern, i would suggest that instead of having background-repeat: no-repeat
, change it to background-repeat: repeat-x;
. this is because the pattern looks awkward when you scale your component to screen widths larger than 1440px
the proceed to payment and cancel order need to be converted to more semantic elements i.e <button> or <a> because presumably, depending on the need of this on a web page, these are going to be doing something
for all interactive elements, make sure to have hover/focus states to indicate that these can be interacted with
i would suggest adding side paddings on your main/body element so that your component isn't hugging the edges.
add an alt attribute to your image
Marked as helpful
@KKonsur
Submitted
@piushbhandari
Posted
i would make the form validation stronger.
when testing i was able to get to the next screen with fxccxvxc@gasd which is obviously not formatted right for an email
Marked as helpful