Expenses Chart Component

Wa alaikum salaam warahmatullah! @hirashabeer

I have read all your questions and inspected your code, after inspection i have found that you just overused css rules. You can improve it by though!

For example, You don't need to use width and height with vw & vh units for components. It's the reason why the component is not responsive for all sorts of devices.

You can wrap the header and main with a div element and remove these following styles,

header {
  width: 43vw;   // instead use - '100%'
  height: 23vh;
  margin-top: 30px; // instead use 'margin-bottom: 1.25rem'
  padding: 0 30px 0px 30px; // instead use '1rem 2rem'
{

main {
    height: 100vh;
    width: 90vw;  // use - '100%'
    margin-top: 30px;
}

• You need to remove width and height rules from media queries also (max-width: 375px).

• Looks like you followed up Desktop First Approach, so that you designed the site for Desktop then you adjusted it for Mobile devices. Trust me this approach itself have a lot of problems and cons.

• Instead you try Mobile First Approach, this approach is better in a way that instead of ignoring the responsive CSS to be added in the end, you have to work on it first, which gives you a responsive site from the beginning.

• Regarding the Logo & Favicon; During inspection i found that the Favicon is working fine no issues with it, but the Logo which want to be displayed on the header is not shown. That's simply because of the source path. You can simply fix it by adding a period (.). Eg: src="./images/logo.svg"

• Additionally you also need to place the footer component without using margin for .attribution. Using margin is not a good solution and also the static layout will cause alignment issues with chart component. Because we need to place it center both horizontally and vertically.

• So you can add position: relative to body and these styles to footer,

footer {
  position: absolute;
  bottom: 1rem;
}

• In JavaScript i noticed that you have selected all popups,

const popups = document.querySelectorAll(".popup");

• But you never used it, instead you select each popup from bar element's previous sibling object.

• Actually you don't need mouseover and mouseout events to trigger those popups, Instead you simply add the values for popups to bar elements which can be encapsulated on data attributes. Then create pseudo elements using :before or :after. Once you have done it you can access it later using attr() function in css.

• At last, can you explain a little bit about this code? For what reason you have added this code? - I think we don't need it.

//   const today = new Date().getDay();

//   if (index !== 6) {
//     index + 1 === today && bar.classList.add("highlight");
//   } else {
//    today === 0 && bar.classList.add("highlight");
//   }

Hope this helps you, looking forward to see your Growth!

Thankyou so much for your time and help @0xabdulkhalid ! The code you asked about at last is what I wrote for making the current day bar blue but this was not working so I commented out to ask the reason... Secondly can you please some resources to learn about the semantic html, accessibility, responsiveness techniques etc. And one last question pleaseee I am thinking of learning react share with me any useful resource

Also @0xabdulkhalid I didn't get your this line You need to remove width and height rules from media queries also (max-width: 375px). ... Secondly in `header { width: 43vw; // instead use - '100%' height: 23vh; margin-top: 30px; // instead use 'margin-bottom: 1.25rem' padding: 0 30px 0px 30px; // instead use '1rem 2rem' {

main { height: 100vh; width: 90vw; // use - '100%' margin-top: 30px; }` If I give it 100% it will consume full width of screen