Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Log in with GitHub
OverviewSolutions17Comments20

All comments

  • tanโ€ข 640

    @tan911

    Posted

    Hello @YossefMagdy,

    Here's my recommendation for improving this app if you have some free time,

    • Keyboard support. User should be able to use keyboard to navigate this app.
    • The image should be positioned so that it is both horizontally and vertically centered.
    • Your app should be responsive.

    Despite this, excellent job on the task!

    Hope this will help improve, Thanks!

    0
  • MANOELA Uriasโ€ข 190

    @Uriasmanu

    Submitted

    Hello everyone, I had a lot of difficulty finding the right logic to group all the results obtained in the interactions. When I finally got the Javascript part right, I realized that there was a problem with the CSS... (You'll find out when the bill is greater than R$ 1000 haha). I need to improve my knowledge in responsive design. I am open to suggestions as I couldn't fix the bill value. Thank you in advance!

    Calculadora de Gorjeta / Tip Calculator

    1

    tanโ€ข 640

    @tan911

    Posted

    Hello @Uriasmanu,

    css:

    • Instead of giving your body element precise width and height definitions, you may utilize the display flex attribute to automatically center the content.
    • Employ specify width in the main class for your cards.
    • You place the button and card using absolute position everywhere. For positioning the content and the button, I advise using flex or grid.

    More info:

    flex and grid

    Hope this will help, Thanks!

    Marked as helpful

    0
  • tanโ€ข 640

    @tan911

    Posted

    Hello @AlexanderMontoya,

    It placed I think bacause you defined your quotation image in an absolute position. The z-index and opacity will not solve the problem unless if you defined your text also. To address your error, you can use the quotation image as background image of your 'testimonial one' class and placed it wherever you want using background-position.

    more info:

    css bacground properties

    Hope this will help, Thanks!

    Marked as helpful

    1
  • P
    Usman Buttโ€ข 60

    @ucod3

    Submitted

    1. What are some best practices for structuring and organizing JavaScript code, particularly when using ES6 modules?
    2. Are there any areas in my HTML or CSS where I could improve the use of semantic elements or the utility-first approach with Tailwind CSS?
    3. How can I further optimize the responsiveness and cross-browser compatibility of this project?
    4. Are there any performance improvements or optimizations that I could implement to make the application run smoother?
    5. Do you have any suggestions for enhancing the user experience or the overall design of the project?

    Interactive Rating Component using Tailwind CSS, Vite & JavaScript

    #tailwind-css#vite

    1

    tanโ€ข 640

    @tan911

    Posted

    ๐Ÿ‘‹Hello, @ucod3, and congratulations on completing this challenge๐ŸŽ‰. Here's my suggestion and to answer some of your questions as well.

    1. In tailwindcss you can refactor your classes by using layers in your main css file, instead of styling your buttons individually with the same classes, just put your one style in 'style.css'.
    2. I think the ratings styles of clicked button will confused your user of selecting it and also the functionality there is broken.

    more info:

    adding custom style in tailwind

    Hope this will help, Thanks!

    1
  • Erel Ropetaโ€ข 385

    @erelropeta

    Submitted

    I just learned how to use the useState, but I wasn't sure if I had used it wisely.

    Questions:

    1. If I have imported a data list (notificationsList.js in this case) in the App.js, can I just pass it on as a prop or is it better to import it on the component? Does having the useState in the App.js as the parent affect this decision?

    I hope someone can review my code which can be found here and let me know how I can improve on using it.

    Thank you so much! :)

    Responsive Notifications Page Using React, CSS Fflexbox, CSS Grid

    #react#bem

    1

    tanโ€ข 640

    @tan911

    Posted

    It's okay to import 'notificationList' directly to the component's who needs it. Also you can pass 'notificationList' as props. However when passing it as props I think it's read not a notificationList, the code looks like this,

    import Header from './components/header/Header';
import Notifications from './components/notifications/Notifications';
import notificationsList from './data/notificationsList';

export default function App() {
  const [read, setRead] = useState(notificationsList);

  return (
    <main className="main">
      <section className="notifications">
        <div className="c-notifications">
          <Header setRead={setRead}  notificationsList={read} />
          <Notifications />
        </div>
      </section>
    </main>
  );
}

    You use 'useState' with initial value of 'notificationsList'. You don't need to pass 'notificationList' itself instead use the 'read' variable to pass data into your children component.

    Marked as helpful

    1
  • Decimo-10โ€ข 120

    @Decimo-10

    Submitted

    • Should I include a <form> element? I didn't put in one, because the page doesn't submit any data from the inputs.

    • Does the "Select Tip %" has to be a <label>. If yes, what should I give for it's for attribute.

    • The input fields are text type. I originally wanted to use number type, but then the spin buttons show up. When I searched for a way to remove it, the only way I found was a non-standard one(::-webkit-outer-spin-button pseudo-element). Is there a proper way to do it?

    • I gave inputmode="numeric" for the input fields, since I couldn't set them to type="number"(because of the spin buttons), but the w3c validator gives a warning since it's not supported by every browser. Is there a better way for this whole number type input field?

    • It's a broad question, but is there any problem with my script? Until now I only wrote 10-20 line scripts with simply one or two event listeners (I tried to provide proper description with comments).

    Thank you for the feedback.

    Responsive tip calculator app

    1

    tanโ€ข 640

    @tan911

    Posted

    Hello @Decimo-10, Here's my suggestion that might improve your applications also to answer some of your questions as well.

    • I think you should, and they should be inside the fieldset if you want to make "Select Tip %" a label. Also you can use input="radio" for every percentage and hide it via visually-hidden class or sr-only class instead of normal button element and don't forget to add label to every input radio button.
    • You can use javascript to handle the validations for the input type text. If you do this, then you have to check if the input value can be converted by a number or not. If it is, do the calculations otherwise just give an error message there. When providing an error message you can add aria-describeby to your input element with the same value to the id of your error message or to your span element. This will benificial to your audience who use screen readers.
    <div class="label-wrapper">
        <label for="bill-input" class="input__label">Bill</label>
        <span id="your-error" class="input__error-message">Can't be zero</span>
      </div>
 <input type="text" id="bill-input"  aria-describedby="your-error" class="input__field" placeholder="0" inputmode="numeric" pattern="[1-9][0-9]*\.?[0-9]*">

    More info about fieldset

    Marked as helpful

    1
  • tanโ€ข 640

    @tan911

    Posted

    Hello @Elomolina, An accordion should have keyboard support when it is built. This is beneficial to your audience since they can now use the keyboard to navigate your app. You can read this guide if you're interested in making an accessible accordion. - Accordion Example WAI ARIA.

    0
  • tanโ€ข 640

    @tan911

    Posted

    Hello @almeida883, Looks good. However, in mobile there's no padding on the right side of your content and the screen.

    0
  • tanโ€ข 640

    @tan911

    Posted

    Hello @LisandraFerraz, your implementation is good because it supports the keyboard. However, try scaling your screen down to 320px, and your application is still not entirely responsive, and the background image is not aligned. Additionally, I am unable to scroll it vertically, you should enable this so that the content of your application is viewable regardless of the audience's screen size (height or width). If you want to improve the accessibility of your application you can refer this guide of building accessible accordion - Accordion example - WAI ARIA

    Marked as helpful

    0
  • tanโ€ข 640

    @tan911

    Posted

    Hello @ViniciusMassari, your application should have a keyboard support, you can use button element instead of dl element. button has accessibility features such as, tabs key, shift+tabs key or you can read this guide of making accessible accordion. When building an accordion try this - Accordion example

    Marked as helpful

    1
  • Arunโ€ข 90

    @ninjabro

    Submitted

    i thought i could finish it faster and stuck took me more than 2hrs, im struggling with placing images in right place with sizes it takes me lot of time. how do you usually place images in flex containers ?

    order summary card

    1

    tanโ€ข 640

    @tan911

    Posted

    Hello @ninjabro, I dont know why you add max width to your body element even if you set display flex the content will not centered on larger screen, try to scale up you screen to 1440px up. You can add max-width to your main element if you want. Also you can wrap your image wih div element instead of header, the image there is just decorative.

    Marked as helpful

    1
  • AndrewFerrer000โ€ข 20

    @AndrewFerrer000

    Submitted

    • I just learned how to use rem and em however I am not really sure where can I put them correctly, in this challenge I just use them everywhere such font-size padding margin and even in width of the div/img. Did I use them correctly?

    QR code component

    1

    tanโ€ข 640

    @tan911

    Posted

    Good work, but I don't know why you set your root element with the font size of 18px. try to change your default font-size in your browser with 1440px screen size up. So in larger screen even if you set your h1 font-size with rem doesn't make any sense. Also, I figure out that your application isn't responsive try to scale down your screen up to 320px. for padding and margin it will depend on you as long as you know the difference between rem and em.

    If you want to set the font size of your root element you can read this guide - https://www.freecodecamp.org/news/override-root-font-size-for-a-better-user-experience/

    you can use em for media queries then rem font size - https://medium.com/zoosk-engineering/to-em-or-not-to-em-that-is-the-media-query-question-22f4a65e9747

    Hope this will help. Thanks.

    Marked as helpful

    0
  • tanโ€ข 640

    @tan911

    Posted

    Hello, @heion31 ๐Ÿ‘‹. Your solution appears to be perfect, however when I reduce the size of my screen to 376px, the layout does not change as expected (it is supposed to do so by 600px for better mobile experience). Your font size is quite small on the widescreen, in order for your audience to read the text, they must modify the browser's default font size which is not a good idea because you didn't allow your content to scroll it vertically.

    Hope this will help. Thanks.

    Marked as helpful

    2
  • tanโ€ข 640

    @tan911

    Posted

    Hello ๐Ÿ‘‹, @danyczech. Congratulations on completing your second task ๐ŸŽ‰.

    • Since you've included background images in your HTML, you might want to try using the picture element. - more info https://www.w3schools.com/html/html_images_picture.asp

    • Adding flexbox to your body element will allow you to center your content. if you do mind, The resulting code will look like: body { display:flex; justify-content: center;align-items: center}

    Hope this will help, Thanks.

    Marked as helpful

    0
  • Ogochukwu Ugorjiโ€ข 30

    @Ogochukwu-ugo

    Submitted

    Please provide feedback on the best way to use the font size specified in the style guide. I tried using the recommended font size for that particular paragraph, which was 15 px, but I had to reduce it because it was too large.

    Responsive QR Code Component

    2

    tanโ€ข 640

    @tan911

    Posted

    Hello, @Ogochukwu-ugo,

    • Try using rem for font-size, use this tool https://nekocalc.com/px-to-rem-converter to convert you px to rem.
    • To remove accessibility report try wrap your content with main element or you can put a role attribute with the value of main in your div element.

    Hope this will help, Thanks.

    Marked as helpful

    0
  • tanโ€ข 640

    @tan911

    Posted

    Hello @nicofercavv-dev, maybe this will improve your code:

    • Don't use h3 without h2 and it's best practices to use heading element in order.
    • Use "rem" for font-sizes instead of pixels, so that your audience can be able to see the text base on their preferred font sizes.

    Hope this will help improve your code, Thanks.

    Marked as helpful

    0
  • JJโ€ข 160

    @JeremyPaymal

    Submitted

    I did this challenge with HTML and CSS only.

    Does it require to use Javascript or a framework like React?

    Can I improve my code as it is now ? I 'd like to optimise it. I'm pretty sure I can reduce the css line.

    Thanks !

    Product Preview

    1

    tanโ€ข 640

    @tan911

    Posted

    Hello @JeremyPaymal, maybe this will help and improve your code:

    • Wrap your content with "main" element or use a "role" attribute for your div with the value of "main", so it should look like this: <div role="main" class="content"></div>. I suggest you should use main element as sematic html for accessibility and best practices.
    • Do not use pixels for font sizes instead use rem for that, so that your audience can be able to see the text base on their preferred font sizes.

    Hope it helps, Thanks.

    Marked as helpful

    0
  • tanโ€ข 640

    @tan911

    Posted

    Hello @Mohsin-93, maybe this will help and improve your code:

    • Wrap your content with "main" element or use a "role" attribute for your div with the value of "main", so it should look like this: <div role="main" class="card"></div>.I suggest you should use main element as sematic html for accessibility and best practices.
    • Use "h1" instead of "h2", you should not skip h1, therefore don't use h2 without h1 and same with h3-h6. - Do not use pixels for font sizes instead use rem for that, so that your audience can be able to see the text base on their preferred font sizes.

    Hope it helps, Thanks.

    Marked as helpful

    1
  • Willie Santosโ€ข 60

    @WillieSantos

    Submitted

    If possible, send feedback on possible adjustments and improvements to this project.

    Component QR Code

    2

    tanโ€ข 640

    @tan911

    Posted

    Hello, @WillieSantos. maybe this will improve your code:

    • Try to wrap your content with a "main" element or put a "role" attribute with the value of "main" of your div element for accessibility purposes, so it should look like this: <div role="main" class="block"></div>.
    • use "rem" instead of "px" for font size, so your audience can be able to see the text base on their preferred font-size.

    Hope it helps, Thanks.

    Marked as helpful

    0
  • tanโ€ข 640

    @tan911

    Posted

    Hello @toshihikotani, maybe this will help and improve your code:

    • Try to wrap your div element with "main" element for accessibility purposes and It's best practices that your page contain a main element.
    • use "h1" instead of "h2"
    • your concern about centering a div content, try to apply flexbox in you body element

    Hope it helps, Thanks.

    Marked as helpful

    0