@RaijinnnSubmitted about 1 year ago
I am still a beginner in coding, hoping for feedback. :)))
P
Amy Spencer
@amyspencerprojectAll comments
- @amyspencerprojectPosted about 1 year ago
Hi Airus,
Nice work!
A few things you might want to look into. You used a form but then used a "click" for your event listener. Try using "submit" instead. This makes the form accessible for keyboard commands not just a mouse click. You would put a
type=submiton the button and then have the event listener on the<form>div.Your error message is not the custom one in the design but the one provided by the browser. To use a custom message you would want to add a "nonvalidate" attribute to your
<form>div and then you would be able to add the color changes to the input field and have a custom message pop up in UI. When you do a custom error message you should also add accessibility attributes to the<p>div. So addingrole="alert"andaria-live="polite"makes your error message much better for screen readers.I noticed you don't use a CSS reset. It is a good idea to use one especially when you start customizing the UI. Andy Bell has a good, https://andy-bell.co.uk/a-modern-css-reset/
Hope this helps you on your coding journey!
Marked as helpful1 - @alibeniaminaliSubmitted over 1 year ago
- Feedback welcome
What did you find difficult while building the project?
- I wasn't able to add the background colour when the input is :checked. I tried using CSS but it didn't work. I am guessing is because I wrapped the input type in a label tag so when the user clicks the number it also checks the input.
@amyspencerprojectPosted over 1 year agoGood Job Ali!
I remember having some of the same issues with the :checked. In the end I took my
<input>out of the<label>div for accessibility reasons. And then just used adjacent sibling combinators to get my :hover and :checked. Like thisinput:checked + label { background-color: var(--light-grey); }Did you try combining the label and input?
label input:checked { background-color: red; }or maybe
label > input:checked { background-color: red; }The Stephanie Eccles article I used as a resource has a solution that also might work for you. And she has the <input> nested into the <label>. Step #4 https://moderncss.dev/pure-css-custom-styled-radio-buttons/
Hope this helps you. Amy
0 - @domdkSubmitted over 1 year ago
I enjoyed my first challenge. The alignment was honestly the most difficult and the the specifity issues that affected the attribution div. In the end I managed to get everything aligned. I also struggled with getting the layout to sit centered in the mobile view, so this aspect is not working.
@amyspencerprojectPosted over 1 year agoGood Job getting everything centered! The mobile view looks centered on my end by the way 🥳
Have you tried using CSS Flex? This makes centering much more manageable.
Happy Coding!!
0 - @amyspencerprojectSubmitted over 1 year ago
Using CSS Grid is still not intuitive for me but getting easier. I know my styling for this challenge is a bit off. I was happy to finally use some JavaScript in a challenge. That part was much easier for me than the styling.
I took accessibility into account and read lots of the comments that Grace made to other peoples solutions. For the email input, the label was not used in the design and I hide it in a way that screen readers will still pick up on. I also put the hero image in an <aside> element. A screen reader will "see" this as a complementary image.
Let me know what you think!
@amyspencerprojectPosted over 1 year agoI updated my JS to put the eventListener on the form instead of the button!
0 - @javascriptor1Submitted over 1 year ago
I completed the challenge for both mobile and desktop version. The biggest difficulty I faced is working with width. I'm still learning about best practices how to handle elements width in the page.
As per challenge style guide , the breakpoint for mobile is 375px. I'm still not sure what does this mean? does it mean the mobile version of design should appear if the device width is less than or == 375px which mean any width above 375px should get the desktop version??
I found some other solutions not helpful in this regard as they set break point for values like 500 or 600px or even more. Personally , I set the break point for 640px which is the sm size for a framework like tailwindcss.
This isse is still not 100% clear for me and I'm unsure about it.
@amyspencerprojectPosted over 1 year agoHi Mohammed,
The design view dimensions are just what the webpage should look like at that specific viewport size. They are not intended to be the breakpoints. I usually set my breakpoints at 960px(60rem) and 640px(40rem). I had the same confusion when I started but read a post in the old FEM Slack channel that cleared it up. Hope this helps.
Happy Coding!!
Marked as helpful0 - @ryanghollandSubmitted over 1 year ago
This was my first project that I consciously tried to approach with a mobile-first design workflow. After finishing the mobile design, I was surprised at how easy it was to get the desktop design working properly with a media query. I also challenged myself by incorporating a little CSS Grid where I would normally be more comfortable with Flexbox.
I had some trouble getting the image to display properly. It took a lot of trial and error. I got it working, but I still don't fully understand why the properties I used worked the way they did.
I ended up using a container div and a background-image property with the image URL (using a media query to change it between desktop/mobile). I used "background-size: cover" and "background-position: center top". Do these properties make sense here or is there a better way to set up the image for this project?
@amyspencerprojectPosted over 1 year agoNice Job!
Alternatively you could use the <picture> element and set the image to change responsively without using a media query. Doing this challenge was how I first learned about <picture>. Using the background in the way you did works perfectly though.
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/picture
Happy Coding!
Marked as helpful1 - @kwaknerSubmitted over 1 year ago
Quite a struggle with making the image responsive for desktop view and my main-container mess up when getting to desktop view.
Please share your views.
@amyspencerprojectPosted over 1 year agoHi Amenyo,
This was a tough challenge for me as well! Good job getting thru it!!
I have two suggestions. First, use a height of 100% to stretch the image to cover the card div in the desktop view. Second, try using "mix-blend-mode: multiply" on the image and then play with the opacity. This will sharpen up the image and make it pop!
Happy Coding 😊 Amy
0 - @av1adSubmitted over 1 year ago
Honestly, this one was like most of the CSS challenges. I do need tips on how I could write my CSS better.
I got really stuck on the annual subscription area, not sure how I could've gone about with that.
@amyspencerprojectPosted over 1 year agoHi Aviad,
I just finished this challenge and also had a hard time figuring out how to get the Annual subscription elements spaced correctly. I used flex-grow on the middle section so that when the viewport is larger the Annual plan text stays closer to the music icon. There is something funky going on with your background image that I can't figure out. It is almost like it is wrapped around so you see a corner of it below the curve of it mid screen. Are you using some kind of preprocessor for you CSS? That might be causing the issue because your code for the background looks the same as mine. You should maybe ask for help on Discord about it.
Hope this helps. Happy Coding!
1
