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

All comments

  • Eric Anderson• 140

    @shinhosuck

    Submitted

    On this project the hardest part was adjusting the height for the image on sign up page/index page when transitioning from mobile to large screen. I eventually figured it out by using flex-box and grid. Also, on success.html on mobile, the success card gave me some issues. As the screen got smaller, the card height would shrink and the card parent container would show with dark background. I fixed this issue with little bit of JavaScript.

    I have some question regarding CSS.

    1. What is the best practice when working with images?
    2. How to organize CSS?
    3. Best practice for naming selectors?

    Thank you very much for your help!

    news-letter-sign-up using CSS and Javascript

    1

    Antonain E.• 220

    @antoebtfr

    Posted

    Great job ! 👍

    • Missing the cursor: pointer on the subscribe button
    • [email protected] is static in the success modal. Try to set it dynamically. Ask me if you need help

    Answering questions

    1. Depends of the project and of the situation
    2. In the project architecture or in a CSS file ?

    Architecture :

    Depends of the structure, frameworks, CMS used but essentially you regroup them in style folder with different files like style/main.css, style/card.css

    Ex : https://github.com/antoebtfr/fM_TODO_list/tree/master/styles

    File :

    No organisation needed, it's not like scss. Just try to not duplicate the selector like .news-container at line 34 and the same .news-container at line 356 so try to search ( with the command ) in the file if there is no selector yet. If there is media queries in the file try to place them at the end but it's not a "rule"

    3.Make it obvious, and use kebab-case

    It triggers me so much to see the "Challenge by" like this. So ugly😂

    0
  • MelodyPatin• 40

    @MelodyPatin

    Submitted

    Hi there! I feel like the result of my challenge is pretty good, however I guess there are still a lot of best practices I don't know about, so if you'd like to review my code and give me a feedback about stuff I could improve to be more efficient I'd be gratefull :)

    Responsive newsletter sign-up form in HTML, CSS & JS

    2

    Antonain E.• 220

    @antoebtfr

    Posted

    Hi, great job !

    The design is respected and the responsive works well 👍

    There are some mistakes :

    • We can't dismiss the message of the success modal
    • The outline on the input should be disabled
    • if(email.includes("@")){ authorizes this eze@fzef@zefz@fe as an email. For the email verification in front-end, specially in JS, you can use this regex /^[a-zA-Z0-9.!#$%&'*+/=?^_`{|}~-]+@[a-zA-Z0-9-]+(?:\.[a-zA-Z0-9-]+)*$/ Link for the regex

    Marked as helpful

    0
  • Antonain E.• 220

    @antoebtfr

    Posted

    Nice work !

    Just the outline of the input to correct and all good 👍

    1
  • Antonain E.• 220

    @antoebtfr

    Posted

    Hi,

    It's a good work but :

    • The avatar's surroundings is missing
    • The HR element is too opaque
    • Active state instead of hover
    • NFT value + time not are not centered vertically

    Do not hesitate to ask for advice

    0
  • Antonain E.• 220

    @antoebtfr

    Posted

    Hi

    It's great work but :

    • The picture and the modal are stretched between 500px et +/- 900px
    • Wrong typography

    Min/Max-width can be useful when you use relatives units like vw/vh to set an absolute limit

    Marked as helpful

    0
  • Antonain E.• 220

    @antoebtfr

    Posted

    Hi

    I'm going to be picky just because depending on the customer some details will pass or not even if it's an excellent work

    Bad points :

    • The modal is not centered vertically
    • We can see the .for-hover background on corners even if it's not in the hover state
    • Not asked but could be nice to set a cursor: pointer on the picture

    Good points :

    • Responsive work well
    • Accessibility with alt attributes
    • The picture is never stretched

    Feel for to ask for any advice or if you don't agree with something I said

    Marked as helpful

    0
  • Antonain E.• 220

    @antoebtfr

    Posted

    Hi

    There are mistakes that I can see

    • Missing hover states
    • PERFUME letter-spacing missing
    • The picture is stretched between 780px and +/- 1300px

    Not asked but implicit :

    • The page doesn't really need to be able to be scrolled

    Still great work the responsive is respected and good usage of flex

    Marked as helpful

    1
  • Antonain E.• 220

    @antoebtfr

    Posted

    Hi,

    The visual is actually fine

    Just a few things :

    • The width of the "Add to cart" button change on hover
    • The shopping cart + the text is not centered
    • Position absolute + fixed value is the not best pratice. Display flex could be better and more responsive (margin could be used to but display flex much better)

    Marked as helpful

    0
  • Nelson Rosario• 50

    @NellyisDevv

    Submitted

    • I found it a bit difficult to find what width would work with mobile view!

    • Is there anything I could have done better with this challange I know there is a bunch of minor things that should be adjusted and changed to make the project more clean and just better across different devices

    Product preview card component

    2

    Antonain E.• 220

    @antoebtfr

    Posted

    Hi,

    Excuse me in advance if I make some English mistakes

    @media only screen and (min-width: 375px) and (max-width: 760px)

    There is two things

    1. I think this exercise wanted to give 375px as the value for mobile devices so it needed to be set as the max-width

    2. With this code, your website reverts to desktop format below the min-width value which is not desirable. ( + the modal is not correct anymore, the white background on the right side disappears )

    For the details :

    The " Add to Cart " button is not centered horizontally

    
.perfume-container .perfume-information button img {
     height: 2vh; 
     align-items: center; (not needed)
     position: relative; (not needed)
    left: 24px;
}

.perfume-container .perfume-information button #text {
    /* position: relative; */ (not needed)
    /* left: 80px; */ (not needed)
    bottom: 3px;
    font-family: "Montserrat", serif;
    font-weight: 600;

.perfume-container .perfume-information button { 
    [...]
    cursor: pointer;
    position: relative; (not needed)
    right: 5px;
   /* New lines */
    justify-content : center;
    align-items: center;

    To simplify responsive, you can use the vw, vh units and max/min-width with absolute value to delimitate if necessary

    Feel free if you want to ask for more tips and your website is pretty good btw !

    0