Skip to content
OverviewSolutions
41
Comments
13
Josh Javier
@joshjavier

All comments

  • P
    Aydan•680
    @AydanKara
    What are you most proud of, and what would you do differently next time?

    ✅ Proud of:

    • Accessibility and Semantic Markup: I made sure to use semantic HTML elements (like <section>, <article>, <h2>, and <button>) along with ARIA attributes to create an inclusive experience.
    • Interactive and Smooth Transitions: Implementing the CSS transition for expanding and collapsing the FAQ sections really enhanced the user experience.
    • Dynamic Icon Updates: Updating the icon (switching from plus to minus) with descriptive alt text provided clear visual and accessible feedback on the accordion's state.

    🔄 What would I do differently next time?:

    • Dynamic Height Calculation: Instead of using a fixed maximum height for the accordion content transition, I would implement a dynamic calculation using JavaScript (e.g., setting max-height based on scrollHeight). This would make the solution more robust for varying content lengths.
    What challenges did you encounter, and how did you overcome them?

    🏗️ Challenges:

    • Balancing Accessibility with Custom Interactions: Ensuring that the interactive elements (especially custom accordion buttons) remained accessible with clear ARIA attributes and keyboard support was a key challenge.

    • Smooth Transitions for Collapsible Content: Animating the height of an element whose content is dynamic (using height: auto) is inherently tricky, as CSS transitions don't work directly on auto values.

    🎉 How I Overcame Them:

    • ✅ Accessibility: I leveraged native elements (like <button>) to handle keyboard interactions and added ARIA properties to communicate the current state to assistive technologies. This minimized the need for custom keyboard event handling.
    • ✅ Transition Issue: I used a CSS trick with max-height and a predefined large value to simulate smooth transitions. Although this approach works, it highlighted the need for dynamic calculations, which is something to explore further in future iterations.
    What specific areas of your project would you like help with?

    • Dynamic Height Calculation for Transitions: I would appreciate suggestions on how to implement a dynamic solution that calculates the actual height of the accordion content (using properties like scrollHeight) to create smoother and more adaptive transitions.

    • Code Refactoring: Feedback on structuring the JavaScript code in a more modular or scalable way would be beneficial. For instance, should I encapsulate the accordion behavior in a class or use a particular design pattern?

    • Cross-Browser Compatibility: Any insights or testing recommendations on ensuring that the CSS transitions and ARIA attributes work seamlessly across different browsers and devices would be valuable.

    FAQ accordion - HTML, CSS, JavaScript

    #accessibility
    1
    Josh Javier•930
    @joshjavier

    Hi Aydan!

    Good job on using semantic HTML elements and making accessibility a priority in your solution. 🤓👍

    Here are some ideas you could apply in this solution or in future ones:

    • Setting a max-height cuts the content when viewed on mobile. In my case, I can only see four lines of text inside each accordion. You can fix this by implementing dynamic height transitions using CSS Grid. Check out Kevin Powell's video on the topic - it will blow your mind 🤯

    • In addition to toggling aria-expanded when the accordion button is pressed, you may also add aria-hidden to the .accordion-content container so that a closed accordion's content is not only hidden visually, but also to screen readers, thus providing a consistent user experience to everyone. You can read more about this topic here.

    • For your icons, I suggest setting aria-hidden="true" and alt="". Why? Since the accordion button already contains text (the questions) then that effectively becomes the label of the button. Screen readers will also see the aria-expanded attribute and let users know that they can expand or collapse the button, so no need to indicate this manually in the icon. For example, when I tested using VoiceOver in my iPhone, the second item reads:

    Is Frontend Mentor free? Expand section. Button. Heading level 2 article. Landmark. Collapsed. Double-tap to expand.

    Setting the icon to aria-hidden="true" and alt="" will remove the "Expand section" phrase, leading to a more concise description.

    You can also look into inlining SVGs for your icons if you want to add simple transitions like animating a plus into a minus and vice versa. Save this article on accessible icon buttons and this video on inline SVGs for future reference!

    Hope that helps. Happy coding!

    Marked as helpful
  • Rizky•480
    @thomasshelbyyy
    What are you most proud of, and what would you do differently next time?

    I was able to make the tricky and quite complicated flip animation, and I improvise by separating the tens and ones section and i will improve the style next time

    Launch Countdown Timer

    1
    Josh Javier•930
    @joshjavier

    Your implementation of the countdown is really neat. You could look into refactoring this into a web component, say <countdown-timer>, to encapsulate the logic in a single file that you can easily reuse in other projects.

    In terms of accessibility, your social links in the footer doesn't have an accessible label, so I recommend adding an aria-label attribute in your <a> tags, or using any of the techniques in the article Accessible Icon Buttons. Make sure to add aria-hidden="true in your SVGs as well.

    Anyway, once you improve the style, your solution would be perfect. Cheers!

    Marked as helpful
  • Josh Javier•930
    @joshjavier

    Hello fellow 11ty fan 😁

    Awesome work on this solution! If you want to gain a deeper understanding of 11ty's features, I suggest checking out the course Learn Eleventy From Scratch. It's a bit outdated, but it still teaches solid principles on how to structure project files (partials, data, etc.) and accessibility.

    Happy coding, my friend!

    Marked as helpful
  • Josh Javier•930
    @joshjavier

    Hello Anthony! I got curious myself and I found this: Solving Sticky Hover States with @media (hover: hover)

    tl;dr You have to move the hover style in a media query so it will only get applied to devices that have hover capability.

    To preserve the glow effect when we actually tap the button on mobile, we need to add a rule that applies the same style to the :active state:

    .btn:active {
box-shadow: 0 0 25px 1px var(--neon-green);
}

@media (hover: hover) {
.btn:hover {
box-shadow: 0 0 25px 1px var(--neon-green);
}
}

    Hope it helps :)

    Marked as helpful
  • Josh Javier•930
    @joshjavier

    Hello @mandriva19!

    First off, the layout looks really great on mobile. But when I switch to my desktop (I have a 1920x1200 monitor) there are white gaps on the side as shown here: https://ibb.co/J5ymzh9

    In this particular case, you can easily fix this by removing the max-width rules in your main wrapper. But usually, the common pattern is to have a .wrapper div inside each of the sections whose sole responsibility is to center the content and limit its width. The background rules will stay on the parent tags unaffected by the wrapper, so the background can stretch to either side of the page. I used this pattern in my Workit landing page solution if you need a reference. :)

    Nice job on making your code well-structured and very readable. One CSS tip I recommend is using logical properties to make your code more DRY. For example:

    padding-top: var(--spacing-20);
padding-bottom: var(--spacing-20);
/*↓*/
padding-block: var(--spacing-20);

/* Also works for margins! */
margin-left: auto;
margin-right: auto;
/*↓*/
margin-inline: auto;

    Hope that helps, happy coding!

    Marked as helpful
  • Josh Javier•930
    @joshjavier

    Hello Rain 👋

    First of all, congrats on submitting your first solution! You're off to a good start; the layout looks good on both mobile and desktop, and your code is well-structured and readable.

    Some ideas on what you could improve:

    • Use semantic HTML tags to improve accessibility and help with SEO. For example, you can put the container div inside a <main> tag and the attribution div inside a <footer> tag.

    • I also recommend adding an H1 because it tells search engines and screen readers what your page is about. In this case, the H1 can be <h1>QR code component</h1>. You can use a .visually-hidden utility class to add the H1 without showing it visually so your code remains accessible without deviating from the design.

    Hope it helps, happy coding!

    Marked as helpful
  • Josh Javier•930
    @joshjavier

    Hello Jeff 👋

    I think using clip-path is a clever way to implement the curve dividers. Another approach is to use inline SVG, which is what I did in my solution. Both approaches look similar, although the "div with clip-path" uses less markup, so I might try this in a future solution ;)

    As for using grid vs flexbox in the numbered cards section, I think either is fine for this particular layout. Personally I used flexbox as well. I did notice some overflow in the tablet view (https://ibb.co/wWp1w3H), which can be fixed by adjusting the position of the pseudo-element:

    .card-title::before {
/* ... */
left: 4rem; // from -1rem
}

    Hope that helps, happy coding!

    Marked as helpful
  • Josh Javier•930
    @joshjavier

    Hello @MrNikaa! I checked your solution on my iPhone SE and indeed the button is placed at the bottom edge of the screen. To fix this, you can position the button relative to the card like this:

    .card {
position: relative;
}

.roll-button {
position: absolute;
bottom: 0;
left: 50%;
transform: translate(-50%, 50%);
/* Remove the margin-bottom */
}

    You can remove the .roll-container which is adding a huge gap at the bottom half of the card. https://ibb.co/HC8Mv8L

    Also, I noticed that you used a lot of media queries to adjust the font size and spacing of multiple elements. In general, avoid doing this as it makes your app more difficult to maintain and debug. Start with a mobile-first approach, and then only add breakpoints when your design "breaks".

    For example, you can avoid setting fixed width and height in multiple breakpoints if you only set a max-width in your card, like this:

    .card {
max-width: 40em;
margin-inline: 1em; /* Add side margins on smaller screens */
padding-bottom: 4em; /* Add enough space to separate the button from the divider */
}

    Last tip: the alt attribute doesn't always have to be a description of the image. If the image is used for decorative purposes only (like the visual divider), the alt attribute should be empty. If the image has a specific function (like the button), then the alt attribute should describe the function of the image, e.g. alt="Generate new advice". For more info, I recommend checking out Alt-texts: The Ultimate Guide and the W3C alt Decision Tree.

    Hope it helps, happy coding!

    Marked as helpful
  • Josh Javier•930
    @joshjavier

    Hello dia!

    I know you submitted this solution 9 months ago, but I'd like to give my take on your question on whether it's alright to position the H1 off-screen.

    I saw your answer to Hyron's comment, and your idea of not using H1 for the card heading is valid. H1 is used to describe the page as a whole - in this case, the page is not about "Improve your front-end skills by building projects" but about a "QR code component". So I think you made the right call here. :)

    In terms of accessibility, nice job using an .offscreen utility class to hide the text for sighted users, but keep it accessible to screen readers. It's similar in function to the CSS pattern .visually-hidden, which is commonly used for accessibility purposes.

    Also, I suggest removing the figcaption element because it's redundant with the alt text of the img element. The .offscreen class only hides an element visually, so screen readers might repeat the phrase "QR code" more than once, which is not pleasant for the user. You can also remove the figure wrapper as well, but that's up to you.

    Hope I was able to add some value. Happy coding!

    Marked as helpful
  • Josh Javier•930
    @joshjavier

    Hello NixyMc! 👋

    Good job on your solution. Your markup is organized and easy to read from a developer's POV. I see you used BEM for classes and CSS custom properties, which definitely helps with code maintainability. 💯

    In terms of keyboard accessibility, I think the social links at the footer can be improved. I'm able to see the correct active state when I hover over the icons, but not when I press the Tab key to cycle through them. This is because you added the hover animation to the icon inside the link, which is not an interactive element. When using the Tab key, the <a> element is being focused, but not the icon inside of it. You can fix this by adding the .social__link selector to your hover styles.

    For example (a bit wordy, but it works):

    .social__link:hover .social__link__icon,
.social__link:hover .social__link__icon::after,
.social__link:focus-visible .social__link__icon,
.social__link:focus-visible .social__link__icon::after {
color: var(--icon-hov);
border-color: var(--icon-hov);
}

    Even better, I recommend checking Sara Soueidan's article on Accessible Icon Buttons for a better implementation you can apply to your projects.

    Hope it helps, happy coding!

    Marked as helpful
  • Josh Javier•930
    @joshjavier

    Hi Carl!

    Good job on your solution. The code for both your HTML and CSS files are well-structured and very readable. I like how you split element attributes into individual lines, which makes it easier to find and change them later on.

    In terms of semantics, consider replacing your <div class="main-content"> with a <main> element. This should fix the accessibility issues in your solution's report, which you should definitely check out.

    For your Add to Cart button, consider using a button tag. It can be tricky when deciding between buttons vs. links, but in general you should try to use <a> tags when you want to send the user to a different page, and <button> tags to trigger an action -- like adding an item to the shopping cart. I think you also forgot to add active/hover styles for the button.

    The <i> wrapper for your icon is unnecessary -- you can remove it. And since your button already has the "Add to Cart" text, the cart icon can be considered as a decorative image. Thus, you should use an empty alt text for the cart icon to follow web accessibility best practices.

    In terms of layout, the desktop view looks good. For mobile, I noticed the card is overflowing vertically because you set height: 100vh on your div.main-content element. In general, avoid using height and use min-height: 100vh instead to fix the responsiveness.

    Happy coding!

    Marked as helpful
  • Josh Javier•930
    @joshjavier

    Hi Adarsh!

    Nice work on your solution. I really like the subtle hover transition you added to the Add to Cart button. :)

    In terms of accessibility, I noticed that you filled in a value for the button icon's alt text. This is repetitive since the button already contains actual text ("Add to Cart"). You can fix this by using an empty alt text (alt=""), which is the best practice according to WCAG. This should also fix the accessibility issue in your solution's report.

    I also noticed that the card gets cut on mobile view because you set a height to the <body> element. You can replace this rule with min-height: 100vh so that the body element can expand beyond 100vh when the content can't fit. You can also add a padding of 1-2em to the <body> element to give the card a little more breathing room.

    Happy coding!

    Marked as helpful
  • Josh Javier•930
    @joshjavier

    Hi Gabriel! :)

    Nice work on your solution. I like that you used descriptive names for classes, which makes the code more readable.

    Syntax-wise, your HTML is okay. You can improve this by using HTML5 semantic tags, e.g., you can use <main> instead of <div class="wrapper"> which is more descriptive (and also solves the accessibility issues that show up in your solution's report).

    For the image, your solution of using two <img> tags and hiding one depending on the viewport size works fine. As an alternative, I recommend looking into the <picture> element, which allows you to specify multiple versions/sizes of an image for different screen sizes. This way, you don't have to write additional media queries. You can check out my solution to see how I implemented it.

    In terms of accessibility, good job adding descriptive alt text to the main images. However, I noticed you also added alt text to the cart icon in the button. It's actually best practice to use an empty alt text attribute for images that are purely decorative, which applies in this case since there's already a text in the button that says "Add to Cart". In fact, screen readers would read this as "cart icon Add to Cart, button" which is repetitive. You can use an empty alt text for the cart icon so that screen readers will read it as "Add to Cart, button" which is better. :)

    Happy coding!

    Marked as helpful