Skip to content
OverviewSolutions
14
Comments
8

Angel M

@csmurilloCalifornia370 points

I’m a mysterious individual who has yet to fill out my bio. One thing’s for certain: I love writing front-end code!

Latest solutions

View more solutions

Latest comments

  • Angel M•370
    @csmurillo

    Hello Alana, First of all good job in completing the Ping Coming Soon challenge. I went through your code and I would suggest the following:

    1. Add this css to your style sheet .icon-container:hover i{color:white;} this will fix the problem that the icon container background turns blue which covers the icon with blue.

    2. For your email verification I would say your implementation is valid. Some people may remove the error when the form is not on focus. So I would do something like this:(feel free to use)

    let subscribeForm = document.getElementById('emailForm')
let errorMsg = document.getElementById('errorMsg')
let emailInput = document.getElementById('email-input')
//things I added (remove comment)
emailInput.addEventListener('blur',()=>{
  emailInput.classList.remove('inputError')
  errorMsg.style.display='none';
});
emailInput.addEventListener('keydown',()=>{
  emailInput.classList.remove('inputError')
  errorMsg.style.display='none';
});
subscribeForm.children[1].addEventListener('blur', (e)=>{
  emailInput.classList.remove('inputError')
  errorMsg.style.display='none';
});
//end of things i added (remove this)

subscribeForm.addEventListener('submit', (e)=>{
    e.preventDefault()
    if(emailInput.value=="" || !(validator.isEmail(emailInput.value))){
        emailInput.classList.add('inputError')
        errorMsg.style.display='block'
    }
    else{
        emailInput.classList.remove('inputError')
        errorMsg.style.display='none'
    }
})

    Also add this to your stylesheet

    .inputError:focus{
    outline: none !important;
    border: 1px solid red !important;
}

    Some things that may be useful: Pixel vs REM:Article

    More on this: Whats your go to way to go about email validation??? Some people may use stackoverflow to find the result to not use the npm packages available. But most people just use email validation npm packages available in their pure js, react, angular, vue app.(In my experience I have seen this)

    Any more questions just reply to this message.

  • Angel M•370
    @csmurillo

    Hello Joshua, Great job in creating this NFT card component. I think their is not much area for improvement for this project but i do suggest the following:

    1. I noticed that you wrap the whole card component inside an section tag, but a more appropriate tag would be the <article></article>. Article tag are used to wrap card components.Article Tag

    2. I think you should add the style cursor: pointer; to the class .overlay of the image.

    Marked as helpful
  • Angel M•370
    @csmurillo

    Hello Jhonatan, I think you did a good job in this project. If you are looking for semantic tags that you could use for this project I recommend wrapping the whole card inside an <article> tag, this is used to wrap around card components. Article Tag

    Also I think you should wrap "0.041 ETH" and "3 days left" inside <p> paragraph tags. It is more appropriate, since it is more clear that it is content describing the Equilbrium card component overall.

    Marked as helpful
  • Angel M•370
    @csmurillo

    Hello Ashwin, Your frontend application looks similar to the solution provided. However, their is a lot of room for improvement. I made a list of things you may want to add next time around:

    1. Wrap all the main content of a particular page with the semantic tag <main></main> Main Tag

    2. Use Flexbox when you want to center things. I look through your code and saw that you used:

    .wrapper {
    ...
    position: absolute;
    top: 50%;
    left: 50%;
    -ms-transform: translate(-50%, -50%);
    transform: translate(-50%, -50%);
}

    you should instead use:

    .wrapper{
    ...
    display:flex;
    justify-content:center;
    align-items:center;
}

    this is the common method to center content, it is easier and more simpler than what you coded.

    3. Stop using margin to position content. Instead use flex box because when you create frontend web application you always want to conserve the web page content flow. Margin should only be used to separate content from one div to the other which is different to using margin to position where content should be in a webpage. When to Use Margin.)

    Flexbox works as follows, when you use display:flex; on a parent container, its children flow side by side. That said:

    In your code you can add flex on the .container:

    .container{
    display:flex;
}

    Also if you want to fix your project use this styles.css: This is your styles.css I just removed all the margin and added flex box where needed and fixed the size of the webpage using width:100%; height:100%; inside html,body. However I still left for you to add padding and fix the content where needed.

    @import url('https://fonts.googleapis.com/css2?family=Montserrat:wght@500;700&display=swap');
@import url('https://fonts.googleapis.com/css2?family=Fraunces:opsz,wght@9..144,700&display=swap');
/* font-family: 'Fraunces', serif; */
* {
    margin: 0;
    padding: 0;
}

html,body{
    background-image: url("vecteezy_banner-template-background-geometri-for-background_12921076.jpg");
    background-size: cover;
    width:100%; height: 100%;
    font-family: 'Montserrat', sans-serif;
}

.wrapper {
    background-color: #ffefd8 ;
    /* width: 90vw; */
    /* height: 80vh; */
    width: 100%; height: 100%;
    display:flex;
    align-items: center;
    justify-content: center;
    box-shadow: 0 4px 8px 0 rgba(0, 0, 0, 0.2), 0 6px 20px 0 rgba(0, 0, 0, 0.19);
}

.container {
    width: 550px;
    height: 450px;
    background-color: #FFFFFF;
    border-radius: 10px; 
    display: flex;
}

img {
   width: 275px;
   height: 450px;
   border-top-left-radius: 10px;
   border-bottom-left-radius: 10px;

}

p {
    font-family:'CustomFont', Montserrat;
    color: #696969;
    font-size: 14px;
    font-weight: 500;
}

h1 {
    font-family: 'CustomFont', Montserrat;
    font-weight: 500, 700;
}

#para {
    color: #696969;
    
}

#price {
    color: #18A558;
    margin-top: 30px;
    font-family: 'Montserrat';
}

#old-price {

}

button {
    width: 215px;
    height: 40px;
    background-color: #18A558;
    border-radius: 8px;
    border: 1px solid #18A558;
    cursor: pointer;
    color: #FFFFFF;
    padding: auto;
}

svg {
    padding: auto;
    margin-right: 12px;
}

    More Stuff that may be useful:

    1. Mobile First Approach Mobile First Articles

    2. Alt In Images: I would suggest reading this article Image Alt

    3. PX vs REM: I would suggest reading this Pixels Vs Rem

    IMPORTED FONT PS: The way you imported the font is wrong. Always remember to select the font weight and add font-family: 'Montserrat', sans-serif; or font-family: 'Fraunces', serif; to the div/ or whatever tag that requires it.

    If you have any questions just reply to this message.

    Marked as helpful
  • Angel M•370
    @csmurillo

    Hello Elvis, I think you are a beginner, I was also once a beginner so I know. Here are my recommendations:

    1. Learn and follow the Mobile First Approach, this will help you so much when building web applications. DONT START BY BUILDING LARGE TO small screens. why? because when you actually build a website you have to be organized and it is more time efficient

    2. Learn semantic HTML. Learn when to use <header>, <nav>, <main>, and more.

    3. Learn to use CssGrid, this is used when certain position of content is to complicated to style with flexbox.

    Now for my two recommendations for this project:

    1. The Header <header> I did not see this tag anywhere in your project. But typically this is to wrap the top most content of the page. Typically a logo and links, hamburger menus.

    For this project the header should wrap the W and the links. Which in your case would be:

    <header>
          <h1>W</h1>
          <h3 class="menu>...</h3>
</header>

    2. The Menu <h3 class="menu">...</h3> should be

    <nav>
          <ul>
                    <li><a>Home</a></li>
                    <li><a>New</a></li>
                    <li><a>Popular</a></li>
                    ...
          </ul>
</nav>

    The <nav> tag is used for to wrap links, and the <ul> is widely used with nav tag since it is a list of links.

  • Angel M•370
    @csmurillo

    Hola Alexander, Primer de todo muy buen trabajo modificando el css grid para hacer este projecto. Pero algunas cosas que vi en tu HTML me parece que puedes modificar para que este project quede al 100%.

    1. Puedes rodear todo el contenido de cada tarjeta testimonial en un tag <article>, este tag es muy usado para contener contenido de tarjetas. (Article Tag MDN)

    2. Otra cosa que me parece que puedes modificar es el <h1>(usando tu ejemplo):(Hgroup Tag MDN)

     <h1 class=" modo-oscuro">
          Daniel Clifford <br>
          <span class=" modo-oscuro">Verified Graduate</span>
</h1>

    un formato mas adecuado es:

    <hgroup> <h1>Daniel Clifford</h1> <p>Verified Graduate</p> </hgroup>

    La manera que lo implementaste esta bien y es muy usado en el web, pero usando el html5 tag <hgroup> es mas appropiado.

    PS: No tienes link de github para ver to codigo, si alguien mas quiere ver tu codigo y darte mas recomendaciones no va a poder ayudarte. Yo use el google dev tools.

    Marked as helpful
View more comments