Skip to content
OverviewSolutions
18
Comments
10
_SabijaThor_
@SasaVatic

All comments

  • _SabijaThor_•355
    @SasaVaticPosted 12 months ago

    Hi Adri, I just wrote one of the possible solutions for picture hover effect with comments so that it is easier to understand. Example JSFiddle link

    Hope it helps.

    Marked as helpful
    0
  • Mostafa Mohi•170
    @mo-oeSubmitted 12 months ago

    I'll just say one thing don't do lazy coding, if you're gonna build something, build right or leave it. I really spent a lot of time refining this code, maybe more time than the first build. I need help please review the code and tell me what's wrong.

    Four card feature section solution built using Gridbox

    1
    _SabijaThor_•355
    @SasaVaticPosted 12 months ago

    Hi Mostafa Mohi, Your solution is very nice. Two things that I would change as if I was one writing css. I Would not use relative position for .feature-logo. You can position it with padding or margin. I would do something like this to escape overflowing:

    .feature-logo {
    margin-left: auto; ---> add
    display: block; ---> add
    /* position: relative; */ ---> delete
    /* top: 20%; */  ---> delete
    /* left: 75%; */ ---> delete
}

    And for div I will exclude fixed height. In this case it is not mandatory. But what if in the future you add some extra text. Text and .feature-logo will overflow their div parent if their content height is larger then parent height.

    div {
    width: 100%;
    max-width: 400px;
    /* height: 250px; */ ---> delete
    padding: 30px;
    border-radius: 5px;
    box-shadow: 10px 4px 30px var(--shadow-color);
}

    That is my brief summary. If you have any more questions feel free to ask. Good luck Mostafa :)

    Marked as helpful
    1
  • _SabijaThor_•355
    @SasaVaticPosted 12 months ago

    Hi Clinton. You can try with this ---> background-position: bottom 190px right -630px; Play with values in media query to adjust bg position. If you want to find out more here are some resources: https://developer.mozilla.org/en-US/docs/Web/CSS/background-position https://css-tricks.com/almanac/properties/b/background-position/ Hope it helps bye :)

    0
  • Thi Lam•70
    @zoetlamSubmitted about 1 year ago

    🤩 Welcome, everyone, to my sign-up form project! This project offers a seamless and user-friendly experience for registration. Users can effortlessly input their email addresses, with the added convenience of a spellcheck feature to avoid any pesky typos. 🛠️ Built with • HTML5 • CSS3 • Vanilla Javascript 😁 Thanks I'll be happy to hear any feedback and advice!🤗 Thank you very much!

    Newsletter sign up form using Flex, absolute Position

    1
    _SabijaThor_•355
    @SasaVaticPosted 12 months ago

    Hi Thi Lam, you have done nice work. I will give you some hints in improving your solution which you can use on any project you encounter. First thing I noticed is that page refresh when you submit the form. This is default behavior with forms which can be prevented using javascript. Also I excluded some of javascript code that is not necessary.

    let emailError = document.querySelector('#email-error');
let emailField = document.querySelector('#email-field');
let form = document.querySelector('form');

// No need for this emailFiled.value property holds value from the input field
// let inputValue = "";
// emailField.addEventListener("input", function(event) {
//     inputValue = event.target.value;
// });

// Prevent form submit so that page does not refresh
form.addEventListener('submit', (e) => {
    e.preventDefault()
})

function emailCheck(e){
    if (!emailField.value.match((/^[A-Za-z\._\-0-9]*[@][A-Za-z]*[\.][a-z]{2,4}$/))){
        emailError.innerHTML = "Valid email required";
    } else {
        emailError.innerHTML = "";
        document.querySelector('#sign-up').classList.add('hidden');
        document.querySelector('#success-popup').classList.remove('hidden');
        // Remove line under since we use emailField.value now
        // document.querySelector('span').innerHTML = inputValue;
        document.querySelector('span').innerHTML = emailField.value;
    }
}

document.querySelector('#dismiss').addEventListener('click', function(){
    document.querySelector('#sign-up').classList.remove('hidden');
    document.querySelector('#success-popup').classList.add('hidden');
})

    Whole submit logic can be written inside form submit event callback function. But if I modify code to much it wont be so clear. Also I have created JSFiddle example of responsive div which uses max-width CSS property. It is much better to use max-width in this case then width because width is more strict and not responsiveness friendly when it is set up using px units. There are some comments in the example so you can more clearly understand why and what. If you want to learn to make more responsive solutions there is Conquering Responsive Layouts course by Kevin Powell. You can find it on Frontend Mentor. It helped me to improve my html and css code. Happy coding Thi Lam 🙂

    Marked as helpful
    0
  • KingMisach•20
    @KingMisachSubmitted about 1 year ago

    I found it difficult to add the image on the add to cart button and also having the rounded borders. I used the rounded border pixels but the images just did not have rounded borders. I am having fun practicing with front end mentor, any feedback is highly welcome and encouraged i want to get better.

    purely HTML and CSS

    1
    _SabijaThor_•355
    @SasaVaticPosted about 1 year ago

    Hi KingMisach, great work you have done here 🙌🏻 and I am happy to improve my own skills by looking into your codebase.

    Here are some suggestion for this and future projects:

    • it is common practice to reset default styles on html elements, people often write this piece of code as the first line of CSS code or below any @import you may have in CSS file. This is useful so you don't have to reset default margins and paddings in every ruleset where you need different spacing then one which is set up on element by default. And border-box is much more responsiveness friendly. Research what is box-sizing and how does it impact layout of the elements on the page and why it comes handy to set it to border-box. It will help you very much in the future. I encourage you to use it but first research!
    *, *::before, *::after {
  margin: 0;
  padding: 0;
  box-sizing: border-box;
}
    • rather then using relative and absolute position to position your wrapper <div> elements on the screen it would be better practice to leave them to their default values and work with display property with values set to flex, block, inline-block... Don't think it is bad to use relative and absolute units they are very very useful CSS properties but you should ask your self when it is right time to use them. If not used in right way they can mess up responsiveness and your layout.
    • rounded border can be set by adding border radius to parent element and if any edge of child element is overflowing parent element you can set overflow: hidden to parent or set same border radius to child element on edges which are overflowing parent element.
    • you can add image to your button by simply putting img element inside your button element or using library for icons like fontawesome via link inside your head tag. But I believe icon is provided with design files and I'm not certain in which format.
    • I will highly encourage you to try Kevin Powell's Conquering Responsive Layouts Course. It will help you so much in structuring and writing your HTML and CSS code. You will learn how to correctly convert design to Responsive Layout using HTML and CSS.
    • and last, I played around with your code and add couple of tweaks to it. It's not perfect but you can play with it by commenting or deleting properties in CSS to see what happens. JSFIddle Product preview card component

    Keep learning and improving you skills. Best wishes on your programming path 😊🙏

    Marked as helpful
    0
  • Cindy Pao•80
    @CinArb2Submitted about 3 years ago

    Hi, I hope you are all doing great.

    Here is my order summary card. I tried to do my best :).

    While I was doing this exercise I got a problem with padding and margins.

    I have read we have to reset the css style. But most of the time I spend it on giving margin and padding again to the elements.

    Do we really need to reset those 2 properties? Or what is your best approach with margins and paddings.

    Also is the semantic correct in my html?

    Any help I will really appreciate it.

    Thanks :)

    Order summary component

    2
    _SabijaThor_•355
    @SasaVaticPosted about 3 years ago

    Hi Cindy, your solution is on point 👌. From semantic point of view for this simple layout this is not necessary but I will do something like this:

    <section class="hero"> -----> section is a wrapper for one or more containers
    <div class="container"> -----> container is a wrapper for hero section rows and determines 
            width of row element
        <div class="row"> -----> row is a wrapper for hero section content, on rows we can set 
            display options such as flex, grid; row can be parent for multiple card elements

            <div class="card"> -----> this is a content element of hero section and parent/wrapper for 
                all card elements
                <div class="card__header">  -----> this is a div for image
                </div>

                <div class="card__content"> -----> this is a wrapper for card content
                    <h3 class="card__heading"></h3>
                    <p class="card__text"></p>

                    <div class="card__order-box"> -----> this is a wrapper for card payment plan content
                       <div class="card__icon"></div>
                       <div class="card__price-box">
                           <h4 class="card__price-heading"></h4>
                           <span class="card__price"></span>
                       </div>
                       <a href="#" class="card__link"></a>
                    </div>

                    <a href="#" class="btn btn--order"></a> -----> buttons are reusable elements for whole 
                        site so in btn class put all styles that are mutual for all buttons and in btn--order 
                        and btn--cancel class put all styles that are unique for that particular button
                    <a href="#" class="btn btn--cancel"></a>
                </div>
            </div>
        </div>
    </div>
</section>
    0
  • _SabijaThor_•355
    @SasaVaticPosted about 3 years ago

    Hi Jenni, I've been playing with your solution a little bit and modified your code along the way and came up with more responsive solution. Your work is good 👏 I just added minor tweaks so you can try to implement it in your code and see what will happen. Keep in mind that in CSS there are many ways to came up with solution. Here are changes I adopted. Good 🤞 and keep 👩‍💻

    body

    • add display: flex; // here I used flex to position container to center of the screen
    • add justify-content: center;
    • add align-items: center;
    • add padding: 2rem;

    .container

    • add flex-direction: column-reverse; // flex reverse so that picture is on the top on smaller screens
    • add max-width: 900px; // container wont be larger than this size
    • add border-radius: 10px; // border radius for container
    • add overflow: hidden; // to hide image edges that are overflowing outside of the container
    • comment out position: absolute;
    • comment out top: 50%;
    • comment out left: 50%;
    • comment out transform: translate(-50%,-50%);

    .leftContainer

    • comment out flex-grow
    • comment out margin: 50px;
    • comment out padding: 0 25px;
    • add text-align: center; // align text on smaller screen to center
    • add padding: 50px; // when you want to make space between div and its content use padding, when you want to make space between two divs(blocks) use margin

    .rightContainer

    • comment out flex-grow

    .list

    • comment out justify content
    • add flex-direction: column; // use flex column to display items verticaly on smaller screens
    • add align-items: center; // align flex items along x axis to center
    • add gap: 2rem; // gap between items in flexbox

    img

    • comment out max-height: 100%;

    • comment out min-width: 100%;

    • add width: 100%; // set img width to 100% of parent element width in this case parent is rightContainer

    • add height: 100%; // set img height to 100% of parent element height in this case leftContainer

    • add media query for larger screens

    • @media screen and (min-width: 768px) {

    • .container {

    •    flex-direction:row; // on larger screens container should display right and left container horizontally so its row

      }

    • .leftContainer {

    •    text-align: left; // text in left container should be aligned left according to design files

    • width: 60%; // in this case 60% works fine because padding is pushing content inside 50px so we give content some space to breathe by setting width to 60% }

    • .list {

    •    flex-direction: row; // on larger screen list items should be displayed horizontally

      } }

    Marked as helpful
    1
  • _SabijaThor_•355
    @SasaVaticPosted over 3 years ago

    You can set parent div of img to be position: relative and img to be position: absolute then use property left: 50%; on img. It will add gap between parent div left edge and picture left edge exactly 50% of parent div width. Now to finally center img horizontally use transform: translateX(-50%) to move picture to left for half it's width. To position img vertically simply use top: property on img element;

    0
  • _SabijaThor_•355
    @SasaVaticPosted over 3 years ago

    I changed your code little bit. Lines that are under comment you don't need. Also I changed in media query min-width: 1320px to min-width: 768px because desktop view should start before. 1320px is to big resolution to start media query for desktop. Padding in .col class was too big so content was overflowing outside of screen in desktop view. Overall good job 👍. Try your code with these values and see why are things happening way they do.

    body { /* margin: 50px 20px; / padding-top: 10vh; ------------------------------------> added font-size: 15px; font-family: "Lexend Deca", sans-serif; background: hsl(0, 0%, 95%); line-height: 1.6em; } .row { / display: inline-block; / padding: 20px 10px; color: hsla(0, 0%, 100%, 0.75); } @media only screen and (min-width: 768px) { .row { display: flex; justify-content: center;
    margin: 0 auto; max-width: 1200px; ------------------------------------> added } .col { /     width: 25%; */

    /* padding: 100px; */ padding: 30px;------------------------------------> added } .sedans { border-radius: 10px 0 0 10px; } .luxury { border-radius: 0 10px 10px 0; } }

    Marked as helpful
    1
  • _SabijaThor_•355
    @SasaVaticPosted over 3 years ago

    You could use forEach method to avoid duplicate code like this:

    button.forEach((btn, index) => btn.addEventListener('click', function() { box[index].classList.toggle("text-visible"); box[index].classList.toggle("text-hide"); head[index].classList.toggle('bold-text'); }));

    so foreach btn in button node list you can access btn element and its index and add click event to it and use index argument to access box and head node list elements with same index as btn which is clicked. I think it will also be better when you use querySelectorAll to use words in plural buttons, boxes, heads for naming your variables. Sorry for bad english, hope it helps.

    Marked as helpful
    1