Skip to content
OverviewSolutions
7
Comments
175
Ivan
@isprutfromua

All comments

  • Ivan•2,610
    @isprutfromua

    Hi there. Good job!

    I have some suggestions for improvement

    • Close the picture by clicking on the background

    • It would be good if the detail page occupied the entire height of the screen, no more

    • The bottom panel twitches when you open the details page

    • Why do you need the index and [index] folders, what is the difference between them? I think folder components would be enough

    • instead of the slide-out.ts animation you could use the built-in fly animation with the value set to y

    I hope my feedback will be useful to you

    Good luck

  • Ivan•2,610
    @isprutfromua

    Hi there

    Please note the variable declarations. It is better not to use var, it will also be better if you declare the types of these elements.

    var intereactiveNumber=document.querySelectorAll(".numbers")!;
let mainDiv=document.querySelector('.main')!;
var submitBtn=document.querySelector('.sb-btn')!;
let updateRatingDiv=document.querySelector('.ratingText')!;
let ratingDiv=document.querySelector('.rate')!;
var ratingPoint: HTMLInputElement;

    It is better not to use any type, especially since you will always have an event target.

    element.addEventListener("click",(e: { target: any; })=>{

    You don't get an input element here, it's just a number

    let targets=e.target.dataset.num as HTMLInputElement;

    that would be enough

    let targets: number = +e.target.dataset.num

    In general, I'll recommend to repeat Javascript Basics Concepts

    REgards

  • Ivan•2,610
    @isprutfromua

    Hi there

    About your question: you can use an empty block with a minimum height, or do the same with the grid areas.

    Regards

  • Ivan•2,610
    @isprutfromua

    Hi there.

    Your work is very similar to the design, it's great!

    But, I think you should make more use of the framework. Here are some of my tips:

    • move the cards to the data and use the loop each .. as. The data can look like this:
    [{stat: '10k +', content: 'companies'}]
    • You can also create individual components. For example - an image

    I hope my feedback will be helpful.

    Good luck and fun coding 🤝⌨️

    Marked as helpful
  • Ivan•2,610
    @isprutfromua

    Hi there! Good job

    I have some recommendations for you

    • you need to stretch the background with background-size: cover

    • in my opinion, the calcTimer function is redundant. Instead, you could use the following construction:

    let diff = dateToLaunch - today;

function calculateTimer() {
.......
}

window.addEventListener('DOMContentLoaded', () => {
  const timerInterval = setInterval(calculateTimer, 1000);

  if (diff <= 0) {   
    clearInterval(timerInterval)
  }
});
    • In each interval tick you declare new variables with functions, this reduces performance
      const days = String(Math.trunc(time / (1000 * 60 * 60 * 24))).padStart(2, 0);
  const hours = String(Math.trunc((time / (1000 * 60 * 60)) % 24)).padStart(2, 0);
  const minutes = String(Math.trunc((time / 1000 / 60) % 60)).padStart(2, 0);
  const seconds = String(Math.trunc((time / 1000) % 60)).padStart(2, 0);

    I would make calculations in separate constants. The date calculation would look like this:

    const DAYS = 1000 * 60 * 60 * 24;
....
const left = {
  DAYS: null
  .....
}
...
calculateTimer() {
....
left.DAYS = Math.trunc(time / DAYS)

daysValue.textContent = days > 10 ? + days : `0${days}` ;   // use ternar because padStart is experimental
}

    I hope my feedback will be useful.

    Good luck and fun coding 🤝⌨️

    Marked as helpful
  • Ivan•2,610
    @isprutfromua

    Hi there. Great work!

    I have a few comments and suggestions:

    • first of all - test your work yourself. If the top of the text is full of text, it will not be visible. Try it yourself. It is better to avoid fixed sizes where content can change

    • is an extremely unreliable selector

    .content section button

    • please set an unique class for the button. also i would add a smooth background color transition when hovering

    • It is better to use such a tag structure

    article
    header
        section
    footer
        section
        section

    I hope my feedback will be useful.

    Good luck and fun coding 🤝⌨️

    Marked as helpful
  • Ivan•2,610
    @isprutfromua

    Hi there. Good job.

    I have some suggestions for improving css code:

    • declaring variables looks pretty ugly. You need to work on naming.
    
:root {
    /* ### Primary */

--very-dark-blue: hsl(233, 47%, 7%); /*(main background)*/
--dark-desaturated-blue : hsl(244, 38%, 16%); /* (card background) */
--soft-violet: hsl(277, 64%, 61%); /* (accent) */

/* ### Neutral */

--white : hsl(0, 0%, 100%); /* (main heading, stats) */
--slightly-transparent-white-main-para : hsla(0, 0%, 100%, 0.75); /* (main paragraph) */
--slightly-transparent-white-stat-head : hsla(0, 0%, 100%, 0.6); /* (stat headings) */
}

    In my opinion, it would be better like this:

    :root {
    --very-dark-blue: 233, 47%, 7%;
    --dark-desaturated-blue: 244, 38%, 16%;
    --soft-violet: 277, 64%, 61%;
    --white : 0, 0%, 100%;

    --color-text: hsla(var(--white), 0.75);
    --color-heading: hsla(var(--white), 0.6);
    --color-accent: var(--soft-violet);

    --bg-body: var(--very-dark-blue);
    --bg-card: var(--dark-desaturated-blue);
}
    • Your effect does not match the design. Instead, you could use a pseudo-element with a gradient background.
    filter:brightness(80%);
    • also, I would advise you to read about css methodology. This will help you better organize your code. In my opinion, this design has only two elements - a card and a picture. That is, in css it will be two components - .card and .image. Further from them you build a tree structure
    card
    -  card-content
        - card-header
        - card-text
        - card-stats
            -card-stat
            .....

    I hope my feedback will be useful.

    Good luck and fun coding 🤝⌨️

    Marked as helpful
  • Ivan•2,610
    @isprutfromua

    Hi there. Good job!

    I have some suggestions for improving the result

    • Always try to use native elements. It will be better if you use a button element instead of a div. So it can be controlled with the keyboard

    • You can also show the menu when you hover the button. With css:

    .user__share-icon:hover + article-card__user__share-socials {
transform: translateY(0);
opacity: 1;
}

    I hope my feedback will be useful.

    Good luck and fun coding 🤝⌨️

    Marked as helpful
  • Ivan•2,610
    @isprutfromua

    Hi there. Your decision looks good, but there are some corrections:

    • Icons are still not displayed. In my opinion, it is better not to rely on third-party libraries, so I always use svg sprite. The icons are always on the server and it will not be such a surprise that a third-party library did not work.

    • making the button an extraneous element is a bad practice, why don't you just use the input type submit or the button type submit?

    • You also need to add an alternative description for the picture. Or hide it with aria-hidden = true.

    • Your site is perfectly responsive, but he lacks padding top

    I hope my comment will be useful

    All the best

    Marked as helpful
  • Ivan•2,610
    @isprutfromua

    Hi there. Good job! Your solution looks very similar to the design.

    I have some suggestions for improvement:

    • set text-transform: uppercase for section headers

    • I think it would be better to import page components out of a config

    import HomeView from ....
....

component: HomeView
    • For a better UI, you can add smooth transitions between pages. For example:
      <transition name="fade">
    ....
  </transition>
    • I think you could simplify your data file. You could export each page individually, instead of importing an entire data file into single file components.

    I hope my feedback will be useful.

    Good luck and fun coding 🤝⌨️

    Marked as helpful
  • Ivan•2,610
    @isprutfromua

    Hi there. Good job! I have a few comments and suggestions:

    • your card starts to break at 1337 pixels, which is not good. It would be better if you added these styles
    .text-container {
  /* padding: 60px 90px 0 60px; */
  padding: 60px 90px;
}

.image-container {
  /* height: 100%; */
}

.image-container img {
  height: 100%;
  object-fit: cover;
}

    So the picture fills the entire space vertically and horizontally.

    • It doesn't make sense to set additional classes for statistics wrappers. They are not unique, so it would be better to use a table element in this case.
    <div class="companies">
...
</div>
<div class="templates">
...
</div>

    I hope my feedback will be useful.

    Good luck and fun coding 🤝⌨️

  • Ivan•2,610
    @isprutfromua

    Hi there! Your solution looks great.

    On my opinion you can improve this things:

    • try to avoid overwrapping. You use 4 elements as a card wrapper : main > section > .container > .cards-container

    Instead of this you can simply set to the body these styles:

    display: grid
place-items: center

    It will be enough.

    • Instead of using a svg image, you can use a background image. You could convert svg to background with SVG encoder.

    • it's a bad idea to use the aside element as a card wrapper. It is more suitable for panels. You can use section instead.

    • There is no reason to complicate selectors here. You can simply use single .stats selector.

    .more-details .stats

    I hope my feedback will be useful.

    Good luck and fun coding 🤝⌨️

    Marked as helpful
  • Ivan•2,610
    @isprutfromua

    Hi there. You did a good job 😎

    keep improving your programming skills🛠️

    your solution looks great, however, if you want to improve it, you can follow these recommendation:

    • it's better to use link <a> tag here:
    <span class="socials__facebook"><ion-icon name="logo-facebook" role="img" class="md hydrated" aria-label="logo facebook"></ion-icon></span>

    • please add some transitions for buttons. You animation will be smoother =) I hope my feedback will be helpful.

    • There is no reason to use paddings for .col-2__description .

    • It'll be better to use link tag instead of the button. Register it's means that you will go to the register page.

    Good luck and fun coding 🤝⌨️

    Marked as helpful
  • Ivan•2,610
    @isprutfromua

    Hi there. You did a good job 😎

    keep improving your programming skills🛠️

    your solution looks great, however, if you want to improve it, you can follow these recommendation:

    				**HTML**

    ✅ Use HTML5 semantic elements. Make sure correct use of the HTML5 semantic elements like: header, footer, main, nav, article, section. It’s will help you to write more structured piece of code.

    ✅ Set a meaningful img alt attribute. It’s best practice for SEO purpose.

    ✅ Avoid complex wrapping. For better performance please tried to avoid unnecessary wrapping. It will create unnecessary node in your HTML tree and reduce performance too.

    ✅ Write Code Comments. It’s best practice to write human-readable code. Tried to comment your block of code. It will help you or any other developer to refactor the piece of code blocks.

    ✅ Do not use divs to create headers and footers – use semantic elements instead. It's advisable to use the <figure> element when adding captions to your images. It is important to use the <figcaption> element along with the <figure> element for it to work.

    ✅ Follow a consistent HTML format. It is important to remain consistent in your HTML style. You can use prettier to help you with that but the goal is to always follow a consistent way you code your markup.

    				**CSS**

    ✅ Use a CSS reset . By default, browsers don’t apply the same default styling to HTML elements, a CSS reset will ensure that all element have no particular style. For example: css-reset

    ✅ Write consistent CSS. At the beginning part of the project you can set some rules for maintain throughout to your entire stylesheet. If you follow the convention or BEM, you’ll be able to write CSS without being afraid of side effects.

    ✅ Avoid Extra Selectors. Adding extra selectors won't bring Armageddon or anything of the sort, but they do keep your CSS from being as simple and clean as possible.

    ✅ Use Clamp . The clamp function make smaller and simple CSS to control element width.

    width: clamp(100px, 50%, 300px);

    ✅ Use CSS Variables . Making the code flexible is equally important so that refactoring won’t be a problem for you. One of the best ways to achieve this is variables.

    I hope my feedback will be helpful. You can mark it as useful if so 👍 it is not difficult for you, but I understand that my efforts have been appreciated

    Good luck and fun coding 🤝⌨️

    Marked as helpful
  • Ivan•2,610
    @isprutfromua

    Hi there. You did a good job 😎

    keep improving your programming skills🛠️

    your solution looks great, however, if you want to improve it, you can follow these recommendation:

    ✅ Use a CSS reset . By default, browsers don’t apply the same default styling to HTML elements, a CSS reset will ensure that all element have no particular style. For example: css-reset

    ✅ Write consistent CSS. At the beginning part of the project you can set some rules for maintain throughout to your entire stylesheet. If you follow the convention or BEM, you’ll be able to write CSS without being afraid of side effects.

    ✅ Use rem’s or em’s. Using rem’s or em’s is more dynamic way instead of using pixels. Try to use rem’s or em’s rather than pixels.

    ✅ Avoid !important. For avoid code collusion and if you don’t want to break normal flow of browser behavior with your CSS don’t ever use !important.

    ✅ Use mobile first development approach. A mobile-first approach to styling means that styles are applied first to mobile devices.

    ✅ Don’t use @import . The @import directive is much slower than the other way to include stylesheets into a html document:

    <link rel='stylesheet' type='text/css' href='a.css'>
<link rel='stylesheet' type='text/css' href='font.css'>

    ✅ Avoid Extra Selectors. Adding extra selectors won't bring Armageddon or anything of the sort, but they do keep your CSS from being as simple and clean as possible.

    ✅ Use Clamp . The clamp function make smaller and simple CSS to control element width.

    width: clamp(100px, 50%, 300px);

    ✅ Use CSS Variables . Making the code flexible is equally important so that refactoring won’t be a problem for you. One of the best ways to achieve this is variables.

    I hope my feedback will be helpful. You can mark it as useful if so 👍 it is not difficult for you, but I understand that my efforts have been appreciated

    Good luck and fun coding 🤝⌨️

    Marked as helpful
  • Ivan•2,610
    @isprutfromua

    Hi there. You did a good job 😎

    keep improving your programming skills🛠️

    your solution looks great, however, if you want to improve it, you can follow these recommendation:

    ✅ Keep using HTML5 validation attributes (required, minLength, pattern etc).

    ✅ Don’t re-implement stuff HTML5 attributes can do for us (no unnecessary JS).

    ✅ Follow a consistent HTML format. It is important to remain consistent in your HTML style. You can use prettier to help you with that but the goal is to always follow a consistent way you code your markup.

    I hope my feedback will be helpful. You can mark it as useful if so 👍 it is not difficult for you, but I understand that my efforts have been appreciated

    Good luck and fun coding 🤝⌨️

    Marked as helpful
  • Ivan•2,610
    @isprutfromua

    Hi there. You did a good job 😎

    keep improving your programming skills🛠️

    your solution looks great, however, if you want to improve it, you can follow these recommendation:

    ✅ Use a CSS reset . By default, browsers don’t apply the same default styling to HTML elements, a CSS reset will ensure that all element have no particular style. For example: css-reset

    ✅ Write consistent CSS. At the beginning part of the project you can set some rules for maintain throughout to your entire stylesheet. If you follow the convention or BEM, you’ll be able to write CSS without being afraid of side effects.

    ✅ Use rem’s or em’s. Using rem’s or em’s is more dynamic way instead of using pixels. Try to use rem’s or em’s rather than pixels.

    ✅ Use mobile first development approach. A mobile-first approach to styling means that styles are applied first to mobile devices.

    ✅ Don’t use @import . The @import directive is much slower than the other way to include stylesheets into a html document:

    <link rel='stylesheet' type='text/css' href='a.css'>
<link rel='stylesheet' type='text/css' href='font.css'>

    ✅ Avoid Extra Selectors. Adding extra selectors won't bring Armageddon or anything of the sort, but they do keep your CSS from being as simple and clean as possible.

    ✅ Use Clamp . The clamp function make smaller and simple CSS to control element width.

    width: clamp(100px, 50%, 300px);

    ✅ Use CSS Variables . Making the code flexible is equally important so that refactoring won’t be a problem for you. One of the best ways to achieve this is variables.

    ✅ use radiobuttons instead of div .

            <div
          v-for="item in scoreList"
          :key="item"
          v-bind:class="{ active: score === item }"
          class="score-card-container"
          @click="setScore(item)"
        >
          <span>{{ item }}</span>
        </div>

    I hope my feedback will be helpful. You can mark it as useful if so 👍 it is not difficult for you, but I understand that my efforts have been appreciated

    Good luck and fun coding 🤝⌨️

    Marked as helpful
  • Ivan•2,610
    @isprutfromua

    Hi there. You did a good job 😎

    keep improving your programming skills🛠️

    your solution looks great, however, if you want to improve it, you can follow these recommendation:

    ✅ Use a CSS reset . By default, browsers don’t apply the same default styling to HTML elements, a CSS reset will ensure that all element have no particular style. For example: css-reset

    ✅ Write consistent CSS. At the beginning part of the project you can set some rules for maintain throughout to your entire stylesheet. If you follow the convention or BEM, you’ll be able to write CSS without being afraid of side effects.

    ✅ Use rem’s or em’s. Using rem’s or em’s is more dynamic way instead of using pixels. Try to use rem’s or em’s rather than pixels.

    ✅ Use Clamp . The clamp function make smaller and simple CSS to control element width.

    width: clamp(100px, 50%, 300px);

    ✅ Use CSS Variables . Making the code flexible is equally important so that refactoring won’t be a problem for you. One of the best ways to achieve this is variables.

    I hope my feedback will be helpful. You can mark it as useful if so 👍 it is not difficult for you, but I understand that my efforts have been appreciated

    Good luck and fun coding 🤝⌨️

    Marked as helpful