Skip to content
OverviewSolutions
43
Comments
413
P
tediko
@tediko

All comments

  • P
    tediko•6,680
    @tedikoPosted 5 months ago

    Hi!

    You should not use create-react-app to setup and start your React projects. It is no longer recommended by React developer team and was removed from the official documentation. CRA has problems with its performance. It is slow and bulky compared to the modern methods. It also is outdated as the dependencies themselves suffer from warnings during installation. There are few alternatives. I personally use Vite, which I recommend.

    We shouldn't manipulate the DOM directly. React is all about being declarative and so manually selecting DOM elements, manipulating them and attaching event listeners like you did in <NavBar> component is not really the React way of doing things. React utilizes a Virtual DOM to optimize updates to the actual DOM do direct manipulating can interfere with this process, leading to inconsistencies between virtual DOM and the actual DOM. This can cause unexpected behavior, visual glitches and bad performance. Additionally, React components have a lifecycle that dictates when they mount, update, and unmount. Directly manipulating the DOM can disrupt this lifecycle, making it difficult for React to manage component states and updates correctly. For instance, if you remove a DOM element directly, React may not be aware of this change, leading to errors trying to re-render that component. Instead of manipulating DOM directly and using useEffect to handle that side effect you should create local state for that component using useState hook and later on whenever your menu is toggled change that state with event handler using setter function.

    const [isMenuOpen, setIsMenuOpen] = useState(true);

function handleToggleMenu() {
    setIsMenuOpen(prevIsMenuOpen => !prevIsMenuOpen);
}

    And with that, you can attach event handler to your menu button like so:

    <button
    className="text-gray-700 focus:outline-none focus:text-blue-600"
    onClick={handleToggleMenu}
>

    And to make it work, you have to conditionally add class names for mobile-menu within your JSX.

    <div
    id="mobile-menu"
    className={`${isMenuActive ? '-translate-y-full translate-y-0' : ''}`}
 >

    Changing isMenuActive state with handleToggleMenu event handler will trigger re-render of the component which will generate new JSX with updated classes.

    In components where you are returning single DOM element you don't have to wrap returned JSX in <></>. <Fragment> lets you group elements without a wrapper node.

    Have fun!

    Marked as helpful
    0
  • Laura Nguyen•130
    @laura-nguyenSubmitted 6 months ago
    What are you most proud of, and what would you do differently next time?

    I utilized grid-template-areas to create the perfect grid layout for the desktop view. I also timed myself while working on this component and was able to complete it in 1.5 hours, which was faster than I had anticipated. I chose to time myself in order to improve my time management skills and practice working under pressure.

    What challenges did you encounter, and how did you overcome them?

    The cards don't have the same height as each other so initially the grid looked very jumbled. I fixed this by setting the height to 100% on the cards.

    What specific areas of your project would you like help with?

    I would love to learn another efficient way to achieve this grid layout.

    Responsive testimonial section using Sass

    #sass/scss
    1
    P
    tediko•6,680
    @tedikoPosted 6 months ago

    Hello @laura-nguyen! The reason why you need to explicitly set height to 100% on the cards is because of how you set your grid container in first place. The fr unit allows us to define grid tracks as fractions of the available space which means if a grid container has 2 rows with 1fr each, they will each take up an equal amount of space, i.e., each row will be 50% of the available space. You're defining grid-template-rows: repeat(2, 1fr); so both of your rows take up equal amount of space, and since your first row card content is bigger than second row cards it will still divide them with equal amout of space leaving second row with cards jumbled as you said. Instead what you want to use is min-content keyword which is the smallest size a box can take without overflowing its content - like so: grid-template-rows: repeat(2, min-content). This way, your second row will be as small as your largest card in that row. Now you can remove height: 100% from .card container and from .card--5 where you set height: 100% and auto within some breakpoint. Last thing you need to do is remove align-items: flex-start because it is doing excatly what you want to avoid. align-items: flex-start aligns items to be flush with the start edge of their cell and instead you want to fill the whole width of the cell with align-items: stretch. But since stretch is default behaviour (align-items: normal) you don't have to explicitly set that, just remove old property.

    Since you're using display: grid for <main> on desktop you should be consistent and also use grid on mobile instead of flex. Why switch between the two if one can do excatly the same?

    Have fun!

    Marked as helpful
    0
  • mardon1dev•60
    @mardon1devSubmitted 9 months ago
    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

    For this challenge I used HTML and CSS.

    1
    P
    tediko•6,680
    @tedikoPosted 9 months ago

    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.
    • Use 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.
    • You shouldn't set height on .card component. Instead let your inner content decide how much space it need and how high your container will be.
    • Text should never be in span or div alone. Always use meaningful element. Change .card-learning and .card-publish to the appropriate elements.
    • Never use pixels for font-size's. Instead use rem which is relative to font-size of the root element (most browsers set it to 16px). Defining your elements font-size in pixels will not respect the user's font-size preferences and therefore your web page will not be user-friendly.
    • Usually line-height is written as a unitless value, like 1.5.

    Have fun!

    Marked as helpful
    4
  • Daniel Wiśniewski•30
    @TheWichaSubmitted 10 months ago
    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

    blog-card

    1
    P
    tediko•6,680
    @tedikoPosted 10 months ago

    Hi Daniel!

    You did great! Here are some tips from me:

    • By default all web browsers apply a certain amount of default styling to HTML documents eg. h1 are larger that h3, links are blue and underlined etc. These browser defaults don't go away when you add your custom stylesheet to a document. Reduce browser inconsistencies in things like default line heights, margins and font sizes of headings, and so on by using CSS Reset (e.g by Andy Bell)
    • Now that you added some CSS Reset your 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.
    • Change body to take 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.
    • Your document lacks landmarks. 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.
    • Since .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.
    • Text should never be in div or span elements alone. Always use a meaningful element. Change .label div and .user-info span to paragraph (<p>) element.
    • Your page is lacking <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
    0
  • P
    Franci Melink•400
    @francimelinkSubmitted 10 months ago
    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

    Blog preview card trainig accessibility and CSS basics

    #accessibility#lighthouse
    1
    P
    tediko•6,680
    @tedikoPosted 10 months ago

    Hello @francimelink!

    Good job on this challenge. Some things I'd change:

    • The <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.
    • You shouldn't define 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.
    • You shouldn't use <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.
    • Author avatar should contain alt attribute since it isn't decorative image. You shouldn't hide it with aria-hidden attribute.
    • Change body to take 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
    1
  • Roksana•330
    @tloxiuSubmitted 10 months ago
    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! :)

    Frontend Development Workflow of Tip Calculator App

    #accessibility
    2
    P
    tediko•6,680
    @tedikoPosted 10 months ago

    Hi @tloxiu!

    Congrats on finishing another project! Here is my feedback:

    • Your logo image conveys no important information necessary for the user to understand the page content so it should be decorative. Keep alt attribute empty or wrap your logo with a element so it point's to home page like your alt text is saying.
    • You should add 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).
    • I am using Firefox and in your inputs there are arrows. Find a way to remove them to match design.
    • I believe using buttons for selecting tip is just wrong. Instead you should use input with type="radio" and the custom tip should be a radio input that when selected reveals a input type="number".
    • It is hard to use your solution with keyboard. First of all you should add some :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 %.
    • There is a bug when i input numbers like: bill: 66, num of people: 3 and click on 15% tip, the output is: $3.3000000000003 and it should be fixed to two points after decimal point.
    • Using the !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
    0
  • Roksana•330
    @tloxiuSubmitted 10 months ago
    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.

    Frontend Development Workflow of Age Calculator App

    #accessibility
    2
    P
    tediko•6,680
    @tedikoPosted 10 months ago

    Hello @tloxiu!

    I can't keep up with your pace! Good job on this challenge. Some things I'd change:

    • Don’t combine landmarks like 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.
    • Wrap your divs that contain inputs and labels with <fieldset> element, which is used to group several controls as well as labels within a form.
    • Instead of hiding your label with .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.
    • Remove 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.
    • Remove 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.
    • You can additionally add h1 element for your app and hide it with .sr-only since your app doesn't have title.
    • Move your styles from 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.
    • Add some class name for input and label div wrappers and style them with max-width: 160px; width: 100%; so they don't grow more than expected in design.
    • Instead of having section for divider and button, move that section within your form (outside of fieldset) and change tag to div. Replace <hr> element with div and add height: 2px; background: your-color.

    Have fun!

    Marked as helpful
    2
  • P
    tediko•6,680
    @tedikoPosted 11 months ago

    Hello @tloxiu!

    Great to see another challenge solution from you! A few suggestions from me:

    • Move 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.
    • Your .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.
    • You should add 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.
    • You're using broad names for color variables within your css. Try to make them more descriptive so you can use them easier within your code. Each color have its own name so instead prefix them with --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.
    • Let's take a look at 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
    1
  • Rory McPherson•70
    @rorymcphersonSubmitted 11 months ago

    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.

    Stats preview card component

    1
    P
    tediko•6,680
    @tedikoPosted 11 months ago

    Hello!

    Congrats on finishing another project! Thanks for providing your thought process aswell as asking detailed questions! Here is my feedback:

    • To me, this image isn't decorative. Anywhere an image adds any kind of value for sighted users, then alternative text should be provided so you should use 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
    • Answering your question why image-color isn't matching design. There should be some opacity on your image to make your blend color brighter. Normally you would put background-color on .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;
    • I wouldn't sweat about making your solution pixel perfect. Of course you want to match design but don't waste your time to move elements one pixel here and there. Later when you'll have access to figma and design files taking those measurements like spacings, sizing etc will be easier so you will match "pixel perfect" design.
    • As for why you struggle with responsivnes. Look at mobile-first approach which is designing a desktop site starting with the mobile version, which is then adapted to larger screens. But since you make the other way around here's why it doesn't work for you. You should change 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.
    • You should definitely take a read how to make your solutions responsive. read more.

    Have fun!

    Marked as helpful
    0
  • P
    tediko•6,680
    @tedikoPosted 11 months ago

    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.
    • Instead of putting image within your button, try to use ::before pseudo-element to add this image to button element.
    • You shouldn't put two images into your HTML and remove one with 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.
    • Don't set explicitly 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%;
    • Now you have to add padding to your body since your container want to stretch full screen width. Add something like padding: 1rem.

    Good luck!

    Marked as helpful
    0
  • P
    tediko•6,680
    @tedikoPosted 11 months ago

    Hello @ArdaBozan! Good job on this challenge! Here's my few suggestions:

    • Don't separate your HTML class names with "|". Separate the class names with a space, e.g. <div class="class1 class2">.
    • I'd use only one paragraph for your .price and .opacity text. Wrap opacity text into span element and style it. This will also get rid of .price-block div.
    • Your <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
    1
  • MonicaPoloni•60
    @MonicaPoloniSubmitted 11 months ago

    Hello, this was my first project. I'm starting with HTML and CSS and would be grateful for any suggestions for improvement. Thanks!

    Social Links Perfil

    4
    P
    tediko•6,680
    @tedikoPosted 11 months ago

    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
    0