I couldn't get the hide/show JavaScript to work for me. If someone could please offer any suggestions or advice? Happy Coding!
Jax Teller
@piushbhandariAll comments
- @Glenda9031Submitted 11 months ago@piushbhandariPosted 11 months ago
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.- in your JS, the class that's being added to the accordion button isn't doing anything. when checking if the element has .title__click, add .title__click to the button AND to the .accordion-text. in your css make sure to have this
.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:
- you can remove the input:checkbox and label. convert the label to a button instead, so
<label for="section1" class="accordion__label title__click" id="title">
will be<button type="button" class="accordion__label title__click" id="title">
- instead of having the svg +/- icons as an :after element, just put them into their own <img/> element instead inside the <button> element when you convert it from your label. use css to hide the - icon. when you attach the .title__click class to your button element use css to show the - icon and hide the + icon. then do the opposite when the .title__click class gets removed. right now when i click on your accordion buttons the + disappears and no - icon.
- for you background image pattern, you can leave the alt attribute as
alt=""
since this is for decorative purposes only. i would also set the image to bewidth: 100%;
so that when you zoom out your layout doesn't look cropped out
hope this helps
Marked as helpful0 - @SebBudynskiSubmitted 11 months ago
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
@piushbhandariPosted 11 months ago- use
.questions:last-child{}
to remove the border + bottom margin/padding from the last question - any reason why
<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 start - assign classes instead of setting inline styles. i.e add active to the answer and give it
display:flex;
- if this
<div class="title">
is being used as your main title replace the the div with <h1>
Marked as helpful1 - use
- @danielmrz-devSubmitted 11 months ago
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. Myindex.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 😊
@piushbhandariPosted 11 months agonice 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
1 -
- @123Joseph-IfeSubmitted 11 months ago
this helped me practice my form validation
@piushbhandariPosted 11 months agoform 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 helpful0 - @ddkogitSubmitted 11 months ago
Is this optimal way to do?? toggling class, or if there is other please feel free to recommend.
@piushbhandariPosted 11 months ago-
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 helpful0 -
- @ObiFaithSubmitted 11 months ago
What did you find difficult while building the project?
- Lack of understanding of event.preventDefault().
- Redirection from the index page to the success page
Do you have any questions about best practices?
- Yes, I will appreciate that
@piushbhandariPosted 11 months ago- i would avoid using cursor: pointer; for inputs and instead use the browser default
- you have a label but it's not hooked up to your email input. since your for attribute on your label is 'email' i would add attributes
id="email" name="email"
. this is for accessibility. when you click on a label, it should then focus on the input it's associated to - you have a second label that i think you can just use the <p> element instead
- right now, after entering a valid email, when i tried to submit your page leads to a 404 page. it looks like you're submitting the form with no url. i would either remove the outer form or use javascript to prevent the form submitting and instead you controlling what it does via code.
let 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 actionMarked as helpful1 - @GiovanniPereira05Submitted 11 months ago
Any feedback is welcome but especially help about mobile border-radius I didn't know how to do it
@piushbhandariPosted 11 months agofor border-radius, you can set it like this:
border-radius: 10px 11px 12px 13px;
top left, top right, bottom right, bottom left in that orderFYI: 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, thenborder-radius: 0 0 5px 5px
for the last oneMarked as helpful1 - @AyouubElbSubmitted 11 months ago@piushbhandariPosted 11 months ago
-
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
0 -
- @luismadfSubmitted 11 months ago
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
@piushbhandariPosted 11 months agoyou 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)
0 - @FluffyboiiiiiiSubmitted 11 months ago
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!
@piushbhandariPosted 11 months agosurround your h3 into a link like so:
<a href="#"><h3>HTML & CSS Foundations</h3></a>
likely will need to do style updates. no JS requiredthis 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 hover1 -
- @lucky-loreSubmitted 11 months ago
-------- 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 🚀 --------
- Current Phase: Completing all Newbie Free projects.
- End of Current Phase: Code revision and optimization (E.g. Componentization/modularization)
- Next Steps: Moving onto Junior, Intermediate, Advanced, and finally the Guru levels.
- Premium Challenges: After completing the free versions, I'll tackle the premium challenges.
- End Phase: Code revision and optimization (E.g. Adding service workers, web workers, optimizing accessibility, refactoring projects to support PWAs)
-------- Repository Overview 📚 --------
The repository is a Lenra Monorepo. Each project in
/packages
is hosted on Vercel. The live versions you can find in theREADME.md
- Project Status: In Progress 🚧
- Number of Challenges Completed: 5
-------- 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 --------
- NextJS
- Zustand
- React Query
- Tailwind CSS
-------- Future Plans 🌟--------
- Code Optimization: Once all Newbie Free challenges are completed, I plan to revisit each project. I'll focus on optimizing the code, implementing Progressive Web Apps (PWA), enhancing accessibility, and general code clean-up.
- Timeframe: Due to my busy schedule, I'm aiming to complete these challenges ASAP. Your understanding and support are much appreciated!
-------- Connect with Me and Support My Journey! 🤝--------
- Star the Repo: If you find my work valuable or interesting, please give it a star! ⭐
- Follow Me on GitHub: Stay updated with my progress and latest projects by following me here.
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! 🙏
@piushbhandariPosted 11 months ago-
for that wavy pattern, i would suggest that instead of having
background-repeat: no-repeat
, change it tobackground-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 helpful1 - @KKonsurSubmitted 11 months ago@piushbhandariPosted 11 months ago
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 helpful0