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
OverviewSolutions28Comments30

All comments

  • Victor• 1,035

    @CodeVee

    Posted

    @faruking Good job. Few pointers

    • Clicking the sun and moon icons break the applications.

    • The App is not responsive on Tablet and Mobile.

    1
  • Victor• 1,035

    @CodeVee

    Posted

    Hi Pavlo. Fantastic job on this task. I also use rem for my responsiveness. I think it's good and less confusing than em. Few Suggestions

    • Wrap your code with a main tag to fix the landmark accessibility issue.

    • Consider using a different element than the ul or use border-top and border-bottom for your li with padding rather than hr.

    I hope you find this helpful

    Marked as helpful

    1
  • Victor• 1,035

    @CodeVee

    Posted

    Hi THIH_NEZZY, Great job on this task. Few Suggestions

    • Wrap your code with a main tag to remove the accessibility errors
    <main> 
   <div class="card">
       ...
   </div>
</main>
    • Remove the images from the span in the price-time div and place them side by side with the span
    <div class="price-time">
    <div class="price">
        <img src="./images/icon-ethereum.svg" alt="eth" />
        <span>0.041 ETH</span>
    </div>
    <div class="time">
        <img src="./images/icon-clock.svg" alt="clock" />
        <span>3 days left</span>
    </div>
</div>

    Also update the CSS to look like

    .price-time {
    display: flex;
    justify-content: space-between;
    margin: 7% auto 0 7%;
}

.price,time {
    display: flex;
    align-items: center;
}

.price,time > img {
    margin-right: 5px;
}

.price>span {
    color: var(--cyan);
}

.time>span {
    color: var(--soft-blue);
}

    • I would recommend you research the use of padding and apply it to the parent card rather than using margin on child elements.

    • To overlay properly would then be easier with absolute positioning and flexbox.

    I hope you find this useful

    1
  • Victor• 1,035

    @CodeVee

    Posted

    Hi Ogunola, Great Job completing this task. Few Suggestions:

    • Wrap your code with the main tag like <main> your code here </main>

    • Set the border and outline to none to remove the white and additional colors from your buttons

    button {
    border: none;
    outline: none;
}
    • For Screens below 375px, I would suggest you don't worry about it.

    I hope you find this helpful

    Marked as helpful

    0
  • Victor• 1,035

    @CodeVee

    Posted

    @Kehinde13 Hi Kehinde Balogun, Great job completing this assignment. Few Suggestions :

    • Wrap your code with a main tag.

    • Replace your h4 with a h1.

    • Try and center your button

    1
  • Dean Hudek• 290

    @deksa89

    Submitted

    It's my second challenge and any feedback is welcome. I had difficult time to implement blue color on image on hover and it doesn't work as it is supposed to. I assume that there is way easier solution. Cheers!

    NFT Card

    2

    Victor• 1,035

    @CodeVee

    Posted

    @deksa89 Hi Dean Hudek, Great Job completing this assignment. Few Suggestions

    • Change the price heading to h2 rather than h3 to clear the accessibility issue.

    • For the hover state. Remove the blue image and set the background of .inner-photo to cyan 00FFF7. Use flex to center both horizontally and vertically the eye SVG.

    0
  • Victor• 1,035

    @CodeVee

    Posted

    @keltech18 Hi Kelvin Ginikanna, Great job completing the assignment. Few Suggestions

    • Replace the div with class main-box with a main tag.

    • Replace p tag first-paragraph with h1

    • Add a title tag between the head tags. Just after meta should be ok.

    0
  • Victor• 1,035

    @CodeVee

    Posted

    @Khaltech99 Hi khaltech99, Great job on this assignment, and the cool animation was a nice touch. Few Suggestions

    • Your container seems too wide. Reduce the width and padding as it seems there is too much space on the right.

    • An active class should be applied to your rating to know the selected option.

    • Use const for the other selectors as they are not reassigned.

    0
  • Saleh• 390

    @Honko-o

    Submitted

    it's my first time Doing Challenge with Jquery nothing fancy I just fetched the Data, I think in vanilla JS it's done with fetch if I am not wrong. any feedback is appreciated.

    HTML SCSS Jquery Ajax

    #accessibility#bem#sass/scss#jquery

    1

    Victor• 1,035

    @CodeVee

    Posted

    @Honko-o Hi Saleh, Great job on this. A few things I would say are

    • Good use of destructuring. Adds Clarity

    • JQuery was a bit of overkill. Still good though.

    • p can be an h1 tag for accessibility.

    • a should be a button. Makes for better Markup

    0
  • hakeem• 30

    @ladking

    Submitted

    While I was coding the javascript, I had an issue with the rating buttons, I tried making sure that when one rating button was clicked, the others would go back to default. but I was not able to figure out how to do it. Another issue i faced was that my rate button can be selected in no particular other and the submit button works regardless if the user had selected a rating button or not.

    I'll really appreciate your feedbacks

    Interactive Rating Component using HTML, CSS Grid, Flexbox, JavaScript

    1

    Victor• 1,035

    @CodeVee

    Posted

    @ladking, Hi Hakeem. It seems you have done this on a private repository. Make it public so that assistance can be offered to you.

    0
  • Victor• 1,035

    @CodeVee

    Posted

    @Chiku100 Hi Abhilash, Great job on this assignment. I have a few suggestions

    • Wrap your card with a main tag. I would suggest swapping the div tag with attribution class for it.
    • for the attribution styling I'd say change it for it to cover the entire screen like
     .attribution {
    display: flex;
    height: 100vh;
    background-color: hsl(212, 45%, 89%);
    justify-content: center;
    align-items: center;
}
    • Use h1 instead of h2

    Marked as helpful

    0
  • Iván De León Lino• 600

    @Lino-OTM

    Submitted

    This was my first time using vanilla JS on my own, I knew what I had to do but before I had to search for methods for my functions. There are some things I could improve such as blocking the submit button until an option has been chosen or maybe another button to come back to the first rating card...

    Any feedback is VERY welcome! :)

    Interactive rating component [Vanilla JS/Sass/BEM]

    #sass/scss#bem

    1

    Victor• 1,035

    @CodeVee

    Posted

    @Lino-OTM Hi Iván De León Lino, Great job on this. My only suggestion would be to use const rather than let for your query selectors as the are not reassigned.

    Marked as helpful

    1
  • Victor• 1,035

    @CodeVee

    Posted

    @cluod-Alfakhre Great job on this. A few things I might suggest adjusting

    • The active button does not change once selected.
    • There is no need for padding on the main hours
    • Your variables were declared with let. Use const since they are never reassigned.
    • The casing for some of your text is mixed up.
    • The background color for the profile card is different
    1
  • Julian• 280

    @Julr09

    Submitted

    Here's my solution using angular, please let me know what you think and what would you do different or better since I'm just learning angular I'm not entirely sure if I'm doing things the right way. Thank you.

    Time Tracking Dashboard Angular

    #angular

    1

    Victor• 1,035

    @CodeVee

    Posted

    @Julr09 Hi Julian. Great job using Angular on this task. I have a few suggestions

    • Your model class should be within the app folder so as not to need default export.
    • You should try to declare more interfaces to avoid using any in your code.
    • Your CSS class naming could be better. Naming a class t might not be descriptive enough.
    • You could place all your configuration information in an array of objects. Thereby all you need to do is loop through the list. Save in Markup and adds more clarity to it.

    Marked as helpful

    1
  • P
    Katrien S• 1,070

    @graficdoctor

    Submitted

    Gosh, I struggled on this one. And also applauded myself various times. I'm delighted with the result, but nevertheless I have a list of mistakes/errors I don't know how to solve. I didn't finish this excercise in one day. I'm working full time, juggling a few extracurricular activities, so time is limited. It made me loose oversight, which I handled by using a to do-list. So this solution in my head, feels messy.

    The issues I didn't manage to solve:

    • The size of the images inside the main grid are off on tablet view. I assumed it had to do with me setting a max-width: 100%. But when I changed that, I still couldn't fix the height. How would I solve this?
    • The nav-toggle didn't wanted to disappear if I would set display:none so I had to use visibility: hidden. But why didn't the display:none not work?
    • The contact-button on the sidebar doesn't want to change its colours on hover. Yet it does on click?

    Responsive layout using CSS Grid & Flexbox & little bit of JS

    2

    Victor• 1,035

    @CodeVee

    Posted

    @graficdoctor. Great job on this project. I have a few comments:

    • There is a need for more spacing between sections
    • Also the grid was switched around. I don't know if that was a creative choice. If it was an issue then I could give a few suggestions on that.
    • The hover is working on chrome
    • There were also alignment issues.
    • The struggle of doing this with a full-time job is highly applauded.
    • I'd recommend using the Figma design and sectioning the project for your to-do list.
    0
  • Victor• 1,035

    @CodeVee

    Posted

    Awesome job. You could increase the size of your cards or reduce the font and image size to create more space

    Marked as helpful

    0
  • Victor• 1,035

    @CodeVee

    Posted

    Awesome job. You could increase the size of your cards or reduce the font and image size to create more space

    Marked as helpful

    0
  • Victor• 1,035

    @CodeVee

    Posted

    Awesome job. In your repo, you used the color value even though you declared the variable in the root . Still great

    0
  • Victor• 1,035

    @CodeVee

    Posted

    Great job on this. Minor typo on the button. Casing issue

    0
  • Atinder• 100

    @atinderbirsin

    Submitted

    Hi,

    This is my 2 challenge , Please let me know if i have to make any changes to improve my skills. Any suggestion would be appreciated

    3-column preview card component

    1

    Victor• 1,035

    @CodeVee

    Posted

    Awesome job. You have a keen eye that will get better with practice.

    1
  • Victor• 1,035

    @CodeVee

    Posted

    Awesome job. My note is

    1. The box shadow on the button is a bit much and should only go down not around.

    Marked as helpful

    1
  • Victor• 1,035

    @CodeVee

    Posted

    Looks Great. Don't know if that's a personal choice for the last main card to span below Full HD screen . Still awesome though

    0
  • Victor• 1,035

    @CodeVee

    Posted

    Great Work. Few Things I noticed:

    1. You could use "cursor: pointer" for your buttons.
    2. Also use the box-shadow property for the main button. 3.Plan Section is missing the gray background

    Marked as helpful

    1
  • Victor• 1,035

    @CodeVee

    Posted

    Good job.

    1. The responsiveness of the page on medium pc screen could use a little adjustment as the cards fall out of place. On mobile there is an overflow too.

    2. I don't think you need absolute positioning on mobile. Try Flexbox

    Marked as helpful

    0