Looking to hire developers?
@tediko
@mardon1dev
Submitted
What are you most proud of, and what would you do differently next time?
This challenge takes time because of the hovering effect and responsive design.
What challenges did you encounter, and how did you overcome them?
Correct placing of contents
What specific areas of your project would you like help with?
Responsive design, I think
Hello @mardon1dev
Congrats on finishing another project! Here is my feedback:
<main> is a block-level element so it takes up horizontal space by default. There is no need to set width: 100vh to it. It stretches 100% wide by default.min-height: 100vh instead height on <main> element. By doing this your element will be at least 100% of the browser height but can be bigger if needed..card component. Instead let your inner content decide how much space it need and how high your container will be.span or div alone. Always use meaningful element. Change .card-learning and .card-publish to the appropriate elements.line-height is written as a unitless value, like 1.5.Have fun!
@TheWicha
Submitted
What are you most proud of, and what would you do differently next time?
ill use tailwind for sure next time
What challenges did you encounter, and how did you overcome them?
none
What specific areas of your project would you like help with?
none in this case
Hi Daniel!
You did great! Here are some tips from me:
body element doesn't have default margin so your .card stick to screen edges on mobile screen size devices. Add padding: 1rem or so to your body.min-height: 100vh. 100vh means that the initial body height will take 100% of the viewport height, whereas the use of min-height instead of height will let the body element grow even more if necessary..card container should be <main> element..article-img is decorative and decorative images do not need to be announced by the screen reader, leave the alt attribute empty alt="" so it will not be announced to the user.div or span elements alone. Always use a meaningful element. Change .label div and .user-info span to paragraph (<p>) element.<h1> element. h1 represent the main heading/subject for the whole app. Also, do not skip heading levels - start with <h1>, then use <h2>, and so on. Either switch your .card h2 to h1 or you can add h1 element for your app and hide it with .sr-only class since your app doesn't have title.Happy coding!
Marked as helpful
@francimelink
Submitted
What are you most proud of, and what would you do differently next time?
Basically, the project was mainly aimed at the basics of HTML structure and CSS itself. Mistakes are still possible as I am basically still a beginner.
What challenges did you encounter, and how did you overcome them?
As I mentioned above, the project served as a basis for me. I had no particular problems with the project itself
What specific areas of your project would you like help with?
I definitely want to progress in HTML structuring itself, because I still often find myself editing the HTML structure itself. I think this would be a good base when I go on to more difficult projects
Hello @francimelink!
Good job on this challenge. Some things I'd change:
<header> should not be included inside the <main> tag. When a <header> is nested in <main> , <article> , or <section> , it just identifies it as the header for that section and isn't a landmark. Adding loads of headers in sub sections or inside landmarks creates a load of unwanted noise for assistive tech users and means you then have to label every section and header.font-size in your body element and certainly not in pixels. Browsers set the HTML font size to 16px by default. Defining your body element font-size in pixels will not respect the user's font-size preferences and therefore your web page will not be user-friendly.<button> for "learning" label. Buttons are to be used for performing an in-page action, trigger some action. This is just some text with styles.alt attribute since it isn't decorative image. You shouldn't hide it with aria-hidden attribute.min-height: 100vh. 100vh means that the initial body height will take 100% of the viewport height, whereas the use of min-height instead of height will let the body element grow even more if necessary.Happy coding!
Marked as helpful
What specific areas of your project would you like help with?
When I checked the preview of phones on my PC in web dev tools everything works fine, but when I am checking it on my phone, elements are broken, and out of their place, I will be looking into this, but if anyone have an idea, I will be happy to listen to that! :)
Hi @tloxiu!
Congrats on finishing another project! Here is my feedback:
alt attribute empty or wrap your logo with a element so it point's to home page like your alt text is saying.text-align: right for your .input-bill and .input-people-number inputs instead of adding odd padding like you did (padding: 0.4rem 0 0.4rem 17rem;). If user input is long enough it will cut it since there is no space (i know it won't be case in this scenario but it is just bad practice).input with type="radio" and the custom tip should be a radio input that when selected reveals a input type="number".:focus-visible styles for focused elements. Then it'd be nice to "calculate" when user click enter in last input field. Now it only calculates when i click on selected tip %.!important rule in CSS is generally considered to be bad practice because it overrides all other styles. You probably did something wrong if you have to use it.Happy coding!
Marked as helpful
What are you most proud of, and what would you do differently next time?
I most proud of making bigger script in js, cause i want to learn it so badd, and I guess I would do differently the years, months and day calculation, maybe with some library the next time.
What challenges did you encounter, and how did you overcome them?
What specific areas of your project would you like help with?
I would like to hear an opinion about the accessibility, exactly the ARIA attributes things, and I would like the help with positioning the button on desktop design properly.
Hello @tloxiu!
I can't keep up with your pace! Good job on this challenge. Some things I'd change:
main with the role attribute. It is redundant. Semantic elements each have an implicit role. A landmark is an abstract role for a section of content that allow assistive technologies to navigate and to find content quickly.<fieldset> element, which is used to group several controls as well as labels within a form..sr-only class, remove your .input-title paragraph and show label instead. That's what they are for in cases like this where you have visible label in design.aria-labelledby in your inputs. This is what label is for. To represent a caption for an item in a user interface which in your case is input element. for attribute in label should always point to id of proper input.aria-labelledby from .result-section section. First of all it doesn't point to anything since you don't have Results-heading id on any of this headings. Adding labels to sections that already have headings creates a load of annoying unnecessary noise. Most screen reader users rely on headings for navigation.h1 is reserved for title of the page only, you shouldn't have multiple h1's on page. I'd wrap these results with one <p> tag instead.h1 element for your app and hide it with .sr-only since your app doesn't have title.form to fieldset and remove max-width. Instead, add that max-width on your .wrapper element like width: 100%; max-width: 720px. Remember to hide default border on fieldset. Remember to remove min-width and max-width from .card since we set that to .wrapper container.div wrappers and style them with max-width: 160px; width: 100%; so they don't grow more than expected in design.div. Replace <hr> element with div and add height: 2px; background: your-color.Have fun!
Marked as helpful
Feedback welcome! (I had a problem with the text in the button in the desktop version.)
Hello @tloxiu!
Great to see another challenge solution from you! A few suggestions from me:
header and footer out of main. The main content area consists of content that is directly related to or expands upon the central topic of a document or an application where header/footer excludes contant that is repeated across a set of documents such as site navigation links, logos, banners etc which means we can consider them as adjacent content..logo is decorative so keep your alt text empty and add aria-label attribute to your anchor instead so non-sighted users know where this link is pointing to. Something like aria-label="Ping - Home" would be nice.label for your input and hide it with some sr-only class. The <label> element represents a caption for your input..social-medias is clearly a navigation. Nav element represents a section of a page whose purpose is to provide navigation links, either within the current document or to other documents which is true in your case. Change div tag to <nav> and next you should wrap your links inside unordered-list <ul>. Add aria-label attributes to your links to announce where those links are pointing to non-sighted users.--c- or --clr- like --c-blue, --c-blue-pale it will be easier for you to write code since if you start typing --c it will display you all colors and then if you have different variables for maybe fonts, background etc they'll be grouped like --c-, --bg-, --font- etc.input and button elements. You don't want to make width of your input using padding. Instead remove all paddings values from .email-input and set it to padding: 1rem for now (you can modify it later to fit design). Next, add max-width: 350px to allow your input to take 100% width and limit it at 350px so it wont grow bigger. Next, remove padding from button for now. You want your button to take 100% height of input, and space that is left within container. To do so, just add flex: 1 property to your button so it stretches full avaible width and height. As you can see now your button lacks of height on mobile resolution. You need to match height of input so this is where you add padding: 1rem just like you add to your input.Have fun!
Marked as helpful
@rorymcpherson
Submitted
For this project I tried to use semantic HTML elements where possible. However, I still used <divs> for the elements containing the statistics and the image. I would appreciate any feedback on my use of semantic elements.
I had a lot of difficulty figuring out how to replicate the image with colour. I eventually found some tutorials on ::before and ::after psuedo-elements and tried using an ::after selector to layer the image with a background color, but I wasn't able to get this working correctly. Later I came across some image-related tutorials using blend-modes and found that by blending a background-color and background-image I was able to get very close to the project example. I think this is a relatively easy way to get this to work, though I could not get the color to match exactly. Any suggestions here would be appreciated.
On a previous project it was suggested that I try using Perfect Pixel to get closer to the example sizes and dimensions, rather than visually best-guessing. While I found this very helpful, I felt I ended up using a lot more pixel sizes instead of responsive sizes to get things to line up. Furthermore, some of the pixel dimensions I found matched the example were odd/random sizes. While I think this project is probably the closest I have gotten to replicating the dimensions of the example images, it felt unnatural to be using such obscure number values sometimes. Any feedback on how I can use responsive design more, how to make better use of Perfect Pixel, or any other advice or best practices when it comes to sizing of elements would be very helpful.
As a final note, I was struggling to get this project to work on a mobile screen size with media queries. I was able to get my card-info section working more-or-less, but I could not get the image to display properly above the card along with some other issues. So I am going to come back to the media queries later.
Thank you in advance.
Hello!
Congrats on finishing another project! Thanks for providing your thought process aswell as asking detailed questions! Here is my feedback:
img element with alt text instead. If the image is in context, and is related to your content or adds something to it it's likely not decorative. read more.card-img and opacity: 0.75 with mix-blend-mode on img but since you went other approach with background-image as you thought your image is decorative you can't put opacity on your .card-img since it will change opacity for both container with background-color and image within. Instead use ::before pseudo-element on .card-img and put your image there so you have both container and image in different elements. Don't forget to add position: relative to .card-img. position: absolute;
content: '';
background-image: url(images/image-header-desktop.jpg);
inset: 0;
mix-blend-mode: multiply;
opacity: 0.75;
order of flex-items when you switch to mobile screen size so your image is first, and content is second. Add order: 1 to .card-info. Next, you need to get rid of those explicitly set width/height values. You never want to do that. You want your content to take as many space as it needs and it will make you container width/height. Add max-width: 1110px; width: 100%; flex-direction: column to your .container and remove width/height. Remove width from .card-info and padding. Then add something like padding: 1rem. Remove width and set some height on your .card-img like height: 400px; width: 100%; so it take space within your container. Add padding: 1rem to your body so your container doesn't stick to device edge of the screen. Remove padding-right from .stats--item. Now you have your mobile version. From there you can make it so it match desktop design.Have fun!
Marked as helpful
Happy coding as always guys and girls! :)
Hi @tloxiu!
Congrats on completing your next challenge! A little feedback from me:
.dice-icon div element should be replaced with button tag. Buttons trigger some action, such as drawing next advice in your case..dice-img image is purely decorative. You should remove alt text from that image since it doesn't add information to the content of a page. Instead add aria-label="generate next advice" to your button to inform users that use some form of assistive technology what kind of behaviour they can expect.button, try to use ::before pseudo-element to add this image to button element.display: none property. It will cause performance issues since both images need to load on each screen size. Instead use <picture> HTML element for responsive images. This element allow to display identical image content, just larger or smaller depending on the device. This helps to improve performance across different devices.width on your .card container. Let your content decide how much space it need. It will make your container more responsive. Use max-width instead to say how big it can grow. Add this to your card: max-width: 33.8rem; width: 100%;padding to your body since your container want to stretch full screen width. Add something like padding: 1rem.Good luck!
Marked as helpful
@ArdaBozan
Submitted
What did you find difficult when creating the project?
I couldn't adjust the text of the third field, otherwise I had no difficulty :)
Which areas of your code are you unsure about?
In the text
Hello @ArdaBozan! Good job on this challenge! Here's my few suggestions:
<div class="class1 class2">..price and .opacity text. Wrap opacity text into span element and style it. This will also get rid of .price-block div.<button> element should be <a> anchor. Links take the user to a new location, such as a new web page or new section of the current page, buttons trigger some action, such as showing content on the page that was previously hidden, playing a video, or submitting a form. To me "Sign up" will take user to new page.Have fun!
Marked as helpful
@MonicaPoloni
Submitted
Hello, this was my first project. I'm starting with HTML and CSS and would be grateful for any suggestions for improvement. Thanks!
Congrats on finishing your first challenge! ๐
I don't know Grid yet so take my advice with a grain of salt but your .card container doesn't stretch to max-width: 375px because when you aligned your grid items in body element with place-content: center your browser implicitly created tracks (both columns and rows) to align your .card to the center of the screen. Since columns were created implicitly they're set to auto which means they take only that much space that your .card container needs. What you need to do is explicitly set grid-template-columns: 1fr in your body so it stretch across whole body width and then use justify-self: center on .card element to align itself to the center of body. Also add width: 100% to your .card container so it stretch to your max-width now.
Your document lacks landmarks, lists and have multiple heading elements that are unnecessary. Landmarks are a group of HTML tags and roles that define different subsections of a website and help navigate through a website. Your .card container should be <main> element, .buttons container could be a <ul> list and <button> elements should be <a> anchors. Links take the user to a new location, such as a new web page or new section of the current page, bButtons trigger some action, such as showing content on the page that was previously hidden, playing a video, or submitting a form. Then keep your <h1> heading as it is your component title, but change <h2> element to <p>.
Have fun!
Marked as helpful
@maishamir
Submitted
Hey everyone! I'm currently working on making my designs more responsive (haven't touched media queries or mobile design yet, but working towards it!) Not entirely sure if the way I'm doing it currently with the specific percentages is the way to go, but any pointers would be wonderful :D
Note: I designed this on a 13-inch MacBook pro and it looks significantly different to the screenshot. Any tips for making it more exact to the given screenshot would be great!
Hi @maishamir!
.card container should be <main> element, .links container could be a <ul> list and <p class="socials"> elements should be <a> anchors. Then keep your <h1> heading as it is your component title, but change <h2> element to <p>.alt attribute for the non-sighted users.width is not required for body element, remove that property from your css rules.min-height: 100vh. 100vh means that the initial body height will take 100% of the viewport height, whereas the use of min-height instead of height will let the body element grow even more if necessary.height on your container. Your content should tell container how tall it can grow itself.% is relative to its parental container, which in your solution is body element. That mean when your .card have width: 28% and I open your website on mobile (e.g 375px wide), your .card component will have width of 100px only, which is not readable nor responsive. Instead use max-width: 390px (or rems if you read about them) and width: 100% so it will respect your parent container width, but never grow more than 390px. It will instantly make your container responsive without media-queries. The same applies for your margins/paddings. Use px or rem values.width and height values like 100px.
Have fun!Marked as helpful
@davidsonaguiar
Submitted
Tips?
Hello @davidsonaguiar,
Congrats on finishing another project! There is a few thing that you can change:
alt attribute with description for the non-sighted users.body and html elements in your CSS? There is no need to apply style rules to both of them, so stick only with body {} selector.width is not required for body element, remove that property from your css rules.justify-content and align-items properties in your body to align your flex items both horizontally and vertically there is no need to add margin: auto to your main element which is now your flex item. Don't Repeat Yourself.<h1> heading which is title of your component but change <h2> to <p> element.<a> so they can point to external website.Good luck!
Marked as helpful
@ArdaBozan
Submitted
What difficulties did you encounter while creating the project?
I didn't encounter any difficulties while creating the project; however, I paid attention to ensuring that my code snippets were organized. I made efforts to make my code understandable and easy to follow. I tried to arrange each code block logically and reduce code repetition to prevent unnecessary redundancies. Thus, I aimed to establish a better foundation for the project's future maintenance and expansion.
Which sections of your code are you unsure about?
The code was quite simple. However, I'd like to do some more work to better organize the HTML and CSS files and reduce code repetition.
Do you have any questions about best practices for the project?
I don't have any specific questions about considering best practices for the project. However, I'm looking for some tips or suggestions to improve the appearance of the buttons for better user interaction.
Hello @ArdaBozan!
Nice work on this challenge! There is some things you can fix:
alt attribute for your img element. It provides alternate text for an image if the image cannot be displayed or is displayed by assistive technologies such as screen readers. It is required.a elements instead of buttons. Buttons are for performing an in-page action and anchors/links are for navigating to a new page.@zacc-anyona
Submitted
What are you most proud of, and what would you do differently next time?
What I am proud of in this project
What I would do different next time
What challenges did you encounter, and how did you overcome them?
Challenges I encountered in this challenge
What specific areas of your project would you like help with?
I need help in the following areas;
Hello @zacc-anyona!
Good job on this challenge! There is a few things that you can improve in your code and learn a thing or two alongside!
font-size in your body element in pixels. You don't have to do it at all. By default, browsers set the HTML font size to 16px. A lot of people change their base text size in browser to make it more accessible for them. Defining your body element font-size in pixels will not respect the user's font-size preferences. Remove font-size from body to use default browser size.<main> element represents the dominant content of a document. That's why you need to wrap your .content div with <main> element. If you want your .content element can be an <article> but it isn't necessary. Change your main element within .content to div. <header> is element that represents a container for introductory content or a set of navigational links. In your example either of these conditions are met so change <header> to normal <div> element.figure element since you misuse it within your header element. The <figure> element represents self-contained content, and <figure-caption> provides the accessible description to that element. Replace it with div element. As for using this figure for the avatar, I think it is a more reasonable choice, but still don't know if I'd use it for that.width: 30% on your .content component, you could use max-width property to set the maximum width of an element.Another exciting challenge, let me know what you think i can improve. I used react for the challenge. I wanted the card to be a component on it's own getting its data through props from the the main parent. The data comes from an api and saved it using the state hook - the value of the state is the one that was received by the card component. But for some reasons that i still need to investigate the received data in the card was always undefined. I guess it is more to do with promises but if anyone can assist me to get around the issue .
Hello, Chamu! ๐
Congrats on finishing another challenge! ๐ Your solution looks very good and also responds well. Here's some feedback:
console.log methods in your code. Get rid of this as it makes the code harder to read.document.querySelector with React. There is only few cases (sometimes there is still a need for manipulating elements directly) where it can be useful but in most cases there is better way to approach that. Instead you can use useRef() hook. Refs are more reliable for getting the specific DOM node you care about. document.querySelector can give unexpected results if there are multiple elements that match the query and if you're writing a component that will show up multiple times in many places this problem will occur, that's why refs become more of a clear-cut winner.themeControl function in header.js component. You used document.querySelector for three elements which is completely unnecessary. As you know React provides a way to inject javascript right into our HTML using something called JSX interpolation. So instead of taking element from DOM with querySelector and then inject some data with .innerHTML or .src methods you should simply create a expression inside your HTML elemetns like so:/* If theme is true return "Dark" if not return "Light". */
<span className="mode__state">{theme ? "Dark" : "Light"}</span>
/* If theme is true return Moon element if not return Sun element. */
<img className="mode__img" src={theme ? Moon : Sun} alt="" />
document.body property. It represents the <body> node of the current document. Along with this, I used conditional (ternary) operator to either add or remove theme-dark class. Now themeControl() look like this: const themeControl = () => {
theme
? document.body.classList.add("theme-dark")
: document.body.classList.remove("theme-dark");
};
themeControl() call from handleClick since you want this to perform everytime your theme change. For this purpose in useEffect with themeControl function, add theme to dependency array. The dependency array in useEffect lets you specify the conditions to trigger it. So in our case we'll watch for every theme change and trigger useEffect only when that value change, like so:useEffect(() => {
themeControl();
}, [theme]);
localStorage save option in header.js. In functional programming it is good to break up your code into small reusable functions. For this, we will create two functions, one to save data to localStorage and one for retrieving that data.postThemeToLocalStorage function where we will set our newTheme value to globalTheme key. Note we don't need to include the window property. You can access built-in properties of the window object without having to type window. prefix. The window object is the top-level object of the object hierarchy. As such, whenever an object method or property is referenced in a script without the object name and dot prefix it is assumed by JavaScript to be a member of the window object. So our function will look like this:const postThemeToLocalStorage = (newTheme) => {
localStorage.setItem("globalTheme", newTheme);
};
getThemeFromLocalStorage function where we want to return our globalTheme key value from localStorage. You don't need to use JSON.parse() method since our value from localStorage is a string, not an object. As the localStorage value cannot be saved as boolean we want to simply convert it. For this, we make a simple comparison, to return either true or false but as boolean values, like so:const getThemeFromLocalStorage = () => {
return localStorage.getItem("globalTheme") === "true";
};
localStorage and savedTheme variables. Then, in handleClick function, instead of manually save our data to localStorage, use postThemeToLocalStorage(!theme). Then, add useEffect hook, that will only trigger on render (by providing empty dependency array) and save our theme from localStorage to theme state, like so:useEffect(() => {
setTheme(getThemeFromLocalStorage());
}, []);
theme state. Since you don't use anything like prefers-color-scheme to detect if the user has requested a light or dark color theme, we assign theme to be false (light theme) - const [theme, setTheme] = useState(false). Previously, when your localStorage was empty you got null value from storage. null is a falsy value so it still works, but we want to avoid such cases.I'll send you my copy of header.js on slack just if I forgot to include here something. Good luck with that, have fun coding! ๐ช
Marked as helpful
@karbowskam
Submitted
I think I did it right :) I did two break points: 375px and 1440px (previously I did one more in the middle).
However, I think I did something wrong with the background. Could someone check it out? :)
Thanks and hugs! :)
Hello, Gutka! ๐
Congrats on finishing another challenge! ๐ Your solution looks good. Here's my few tips:
background-repeat approach in your css. To do this you'll have to create empty div and remove all img's.Good luck with that, have fun coding! ๐ช
Marked as helpful
@majxla
Submitted
How to smoothly scale an image as the screen width changes?
Hello, Maja! ๐
Congrats on finishing another challenge! ๐ Your solution looks good. Here's my few tips:
<ul> for .stats. HTML lists are used to present list of information in well formed and semantic way.mix-blend-mode: multiply with opacity: 0.75 on your .overlay container. The mix-blend-mode sets how an element's content should blend with the content of the element's parent and the element's background.CTRL+SHIFT+M on Firefox, and CTRL+SHIFT+C on Chrome. On mobile resolution (375px) it doesn't seems good. Change padding-left and padding-right to 20px on mobile..attribution overflow content on mobile resolutions.height of .image to 240px to make it a bit smaller.Good luck with that, have fun coding! ๐ช
@Alex-AR-LR
Submitted
I would appreciate any help to keep improving
Hello, E_X_E! ๐
Good effort on this challenge! ๐ Everything works good and responds well. Take a look at:
:focus pseudo class to interactive elements like anchors, buttons etc. Use outline property to make your website more accessible to keyboard users. Focusable elements like anchor, buttons or inputs they have applied default :focus pseudo class with outline property. These default styles are subtle and hardly visible tho. Furthermore every browser has a slightly different default style for the outline, so you probably want to change the default style. Read more about why we should change focus styles.alt attribute for the .card__img, as it doesn't add any extra context for screen reader users. Since your image is decorative your alt text should be provided empty (alt="") so that they can be ignored by assistive technologies.alt attribute for header__link logo. "logo" is not descriptive, and since you wrap your image with anchor tag and it direct to home page maybe change alternative text to "Loopstudios - home page".Good luck with that, have fun coding! ๐ช
Marked as helpful
@Caiqueoak
Submitted
I couldn't match the color of "why us" section.
Hello, Caique Carvalho da Silva! ๐
Good job on this one! ๐ Your solution responds well and overall looks good. Here's my suggestions:
div element has no special meaning at all. Use of more appropriate elements instead of the div element leads to better accessibility for readers and easier maintainability for authors. For .header-title use heading <h1> - <h6> element. For .header-subtitle you can use paragraph <p> element. This also applies to text in other sections.width: 100% to .main-container and remove max-width: 400px so your container will stretch across different screen sizes. Keep max-width: 660px on 660px media query to prevent it for stretching too much.<ul> for .reasons-list. HTML lists are used to present list of information in well formed and semantic way.Good luck with that, have fun coding! ๐ช
Marked as helpful
@SeyBoo
Submitted
Hey, i'm news to js any feedback is welcome thx u :).
Hello, Chevalier Antoine! ๐
Congrats on finishing another challenge! ๐ Your solution looks very good and also responds well. Here's my few tips:
margin on mobile media query to your .container to make some room between edges of page and your container. (e.g. margin: 0 12px)toFixed() method to format your total_some, tip_total values so it doesn't return values like: 13.3335534. You have to convert them to number first, with parseFloat for example.total.innerHTML = "$" + parseFloat(total_some).toFixed(2);
tip.innerHTML = "$" + parseFloat(tip_total).toFixed(2);
setInterval with function that wraps all your logic. What it does is rerun this function every one seccond which is really bad. You did it because you want your calculate() function to fire on each user change. Instead add addEventListener to your input and watch for change event. Then call calculate() function.Good luck with that, have fun coding! ๐ช
Marked as helpful
@Junjiequan
Submitted
No specific questions.
I will deal with HTML validation issues later. I am open to all feedbacks & criticisms, thanks.
Hello, Jay! ๐
Congrats on finishing GURU challenge! ๐ Forgive me for speaking so late, but I had a few days off. Your solution is flawless! I really like the animations you added. They look very smooth and the whole UI is great.
aria-labels in your header navigation links. It seems to me that they're self-descriptive.aria-label to button and keep my image alt text empty. Also change this text to something more descriptive - 'cart icon image' doesn't speak much.Good luck with that, have fun coding! ๐ช
Marked as helpful
@DiuGach
Submitted
Hi gorgeous friends , Diu Gach here, hope you are enjoying the coding process. just wanna let you see my third project with scss and BEM, this time I have seen some few improvements in handling the markup and alignments with flexbox, so it's exciting ๐, kindly have a look on my code (Html,sass and live site) and give me some feedbacks about:
1 - Code design 2 - BEM 3 - Html markup 4 - and about everything that you can advice me with, I will be happy to hear ๐ your feedback folks, thanks.
Hello, Diu Gach! ๐
Good job on this one! ๐ Your solution responds well and overall looks good. Here's my suggestions:
:focus pseudo class to interactive elements like anchors, buttons etc. Use outline property to make your website more accessible to keyboard users. Focusable elements like anchor, buttons or inputs they have applied default :focus pseudo class with outline property. These default styles are subtle and hardly visible tho. Furthermore every browser has a slightly different default style for the outline, so you probably want to change the default style. Read more about why we should change focus styles.alt attributes for the .header__logo, .intro__img, .features__icon, as they don't add any extra context for screen reader users. Since your images are decorative your alt text should be provided empty (alt="") so that they can be ignored by assistive technologies.Good luck with that, have fun coding! ๐ช
Marked as helpful
@benjoquilario
Submitted
This is my 14th fem challenge. I got headache for positioning in this challenge but I really learn a lot. I add a simple animation in this project, I hope I got feedback and suggestion thanks in advance.
Hello, Benjo Quilario! ๐
Congrats on finishing another challenge! ๐ Your solution looks very good and also responds well. I really like the initial animation you added. Here's my few tips:
alt attributes for the .hero__img, as it doesn't add any extra context for screen reader users. Since your image is decorative your alt text should be provided empty (alt="") so that they can be ignored by assistive technologies. :focus pseudo class to interactive elements like anchors, buttons etc. Use outline property to make your website more accessible to keyboard users. Focusable elements like anchor, buttons or inputs they have applied default :focus pseudo class with outline property. These default styles are subtle and hardly visible tho. Furthermore every browser has a slightly different default style for the outline, so you probably want to change the default style. Read more about why we should change focus styles.Good luck with that, have fun coding! ๐ช
BTW - The homepage is blank as this wasn't really included in the design. You'll have to click through to a page to see the content.
This was a big step up for me in terms of the tech used to build the site. Any comments or feedback would be most welcome. I put some details and questions in the readme, but I would really like to know:
Is it OK to use inline styles to set custom properties? I did this using nunjunks and it worked really nicely, but I wasn't sure if this is really best practice.
How to make individual HTML pages from a JSON object! I can see that 11ty's pagination should do this but I couldn't make it work!
Thanks for any other comments and feedback!
Hello, Dave! ๐
Congrats on finishing another challenge! ๐ Your solution looks good and also responds well. I have also finished this project recently if you'd like to see. Here's my few tips:
overflow: hidden on the <body> element. (btw. now I've noticed that I've also forgotten about it! ๐
)li element so when I use my keyboard to navigate my page the focus isn't covering whole element but just an anchor.font-sizes etc. It seems big now :DGood luck with that, have fun coding! ๐ช
Marked as helpful