Skip to content
OverviewSolutions
13
Comments
18
P
Jacksen
@jacksen30

All comments

  • Jacksen•350
    @jacksen30Posted about 1 year ago

    Hello @samoina,

    Congratulations on completing this challenge! You've done a fantastic job. The image overlay on hover makes this task more intricate than it initially seems.

    I noticed you submitted your solution a while ago without receiving any feedback. It's unfortunate to see a decline in the number of people offering feedback and advice in this forum.

    Turning to your code, a minor discrepancy caught my eye: the child elements within .main__avatar aren't flush with the left edge of the card, unlike the other elements. Since you've already configured it with display: flex, rectifying this is straightforward. Simply specify width: 100% to ensure this div mirrors the size of its parent component, excluding the padding.

    Additionally, there seems to be a tad too much padding on the card. Reducing the padding should bring it closer to the intended design:

    .main {
   padding: 1.2rem; 
}

    Something more to think about - CSS Variables Should Convey Purpose:

    Adopting meaningful names for CSS variables not only adds code clarity but also fosters design flexibility and better collaboration. Naming conventions based on appearance, like --main--very-dark-blue, don't convey their intended application or adjust gracefully with design alterations. Context-based naming, for example, --primary-background, offers more intuitive understanding and provides flexibility in Theming if you decide to change your design theme later, and the primary background color is updated.

    I, too, often overlook this aspect, but it's a valuable point to bear in mind for the future.

    I hope you find these suggestions helpful! Let me know if you have any questions or if there's anything else I can assist with. Keep up the great work ! 🚀🌟😊

    0
  • Jacksen•350
    @jacksen30Posted about 1 year ago

    Hello @samoina,

    Firstly, great job on the code! It's evident that you've put great effort and thought into this, it really shows. 😃

    I had to have a really good look to find where I could offer some constructive and helpful feedback. Below are a couple of areas I found that could benefit from some tweaks:

    Button Hover Issue:

    There seems to be a layout shift when hovering over the button due to the border being added. A simple fix is to move or include the same border width in the button's base state to maintain size consistency.

    Original Code:

    button {
	font: inherit;
	padding: 1rem;
	border: none;
	border-radius: 3rem;
	background-color: var(--very-light-gray);
}

button:hover {
	border: 2px solid white;
	cursor: pointer;
	color: var(--very-light-gray);
}

    Refactored To Fix layout height change on button:hover :

    button {
	font: inherit;
	padding: 0.5rem 1rem;       /* Similar To The Design */
	border: 2px solid white;    /* Moved from :hover to base state */
	border-radius: 3rem;
	background-color: var(--very-light-gray);
}

button:hover {
        cursor: pointer;
	color: var(--very-light-gray);
        background-color: inherit;
}

    CSS Refactoring for Button Hover Background Color:

    Rather than setting individual background colors for each button on hover, it's possible to streamline the code with inheritance. This can help reduce redundancy.

    Original Code:

    .button__sedans:hover {
	background-color: var(--bright-orange);
}

.button__suvs:hover {
	background-color: var(--dark-cyan);
}

.button__luxury:hover {
	background-color: var(--very-dark-cyan);
}

    Can be refactored down to:

    .button:hover {
    background-color: inherit;
}

    Again, well done on the coding. These are just some suggestions to further refine it. Looking forward to seeing more of your work in the future! 💻

    0
  • Jacksen•350
    @jacksen30Posted about 1 year ago

    Hey @leodk293,

    Good effort! The solution definitely requires a bit more fine-tuning, though. I'd recommend starting off with some of the more basic challenges first. Here are some points to consider:

    HTML Structure and Nesting:

    The <main> tag must be nested within the <body> tag. Currently, it's outside of it. Remember, all the content of the page goes inside the <body> tag.

    Use of Headings:

    Instead of using generic div tags for the titles like 'SEDANS', 'SUVS', and 'LUXURY', consider using semantic heading tags. This could be <h1>, <h2>, or <h3>, depending on the content hierarchy.

    Semantic Tags:

    Utilize semantic HTML tags where appropriate. This enhances both accessibility and the meaning of the content. For instance, the description of each car type can be wrapped in a <p> (paragraph) tag. Additionally, if "Learn More" is intended to be a link, you should use the <a> tag.

    Line Breaks:

    Be cautious with the use of <br> tags for content formatting. They should be used sparingly. Instead, try to control content presentation and spacing using CSS.

    I've noticed a lot of redundancy with your CSS code, when multiple classes or elements require the same property and values it best to create another generic class for that element for example="container1 container",

    please note below I only consider reducing redundancy not fixing the multiple other issues that should be addressed.

    Here is your original code:

    body{
    display: flex;
    justify-content:  center;
}
.overall{
    display: flex;
    flex-direction: row;
}
.container1{
    padding-left: 20px;
    height: 400px;
    width: 300px;
    display: flex;
    justify-content: space-around;
    flex-direction: column;
    border: 1px solid orange;
    background-color: orange;
}
.container2{
    padding-left: 20px;
    height: 400px;
    width: 300px;
    display: flex;
    justify-content: space-around;
    flex-direction: column;
    border: 1px solid #207178;
    background-color: #207178;
}
.container3{
    padding-left: 20px;
    height: 400px;
    width: 300px;
    display: flex;
    justify-content: space-around;
    flex-direction: column;
    border: 1px solid #0A5054;
    background-color: #0A5054;
}

.item1,.item4, .item7{
    color: #EFEEEE;
    font-size: 30px;
}
.item3{
    text-align: center;
    width: 130px;
    height: 35px;
    border:  1px solid white;
    background-color: white;
    color: orange;
    border-radius: 15px;
}
.item6{
    text-align: center;
    width: 130px;
    height: 35px;
    border:  1px solid white;
    background-color: white;
    color: #207178;
    border-radius: 15px;
}
.item9{
    text-align: center;
    width: 130px;
    height: 35px;
    border: 1px solid white;
    background-color: white;
    color: #0A5054;
    border-radius: 15px;
}
a{
    font-size: 30px;
}
.item2,.item5, .item8{
    color: #ECEBEB;
}
@media screen and (max-width:990px){
    .overall{
        flex-direction: column;
        height: auto;
    }
}

    Here is a refactored version that moves a lot of the container and item properties and values to a generic class as to avoid unnecessary duplication of CSS rules:

    body {
    display: flex;
    justify-content:  center;
}
.overall {
    display: flex;
    flex-direction: row;
}

.container {
    padding-left: 20px;
    height: 400px;
    width: 300px;
    display: flex;
    justify-content: space-around;
    flex-direction: column;
}

.container1 {
    background-color: orange;
}

.container2 {
    background-color: #207178;
}

.container3 {
    background-color: #0A5054;
}

.item1,.item4, .item7 {
    color: #EFEEEE;
    font-size: 30px;
}

.item {
    text-align: center;
    width: 130px;
    height: 35px;
    border:  1px solid white;
    background-color: white;
    border-radius: 15px;
}

.item3 {
    color: orange;   
}

.item6 { 
    color: #207178;
}
.item9 {
    color: #0A5054;
}

a {
    font-size: 30px;
}
.item2,.item5, .item8{
    color: #ECEBEB;
}
@media screen and (max-width:990px){
    .overall{
        flex-direction: column;
        height: auto;
    }
}

    If you want to start on some of the more simple challenges I'll be more than happy to provide more detailed feedback on specific pieces of code to help you improve, as you work up to the more difficult challenges. I hope you find my consideration points helpful ☺️

    0
  • Jacksen•350
    @jacksen30Posted about 1 year ago

    Hey @Mtalafa great work on keeping up the consistency on submitting solutions, the code for this looks super clean congratulations ! Getting to good to quick I wont have any feedback to give soon 😃

    Here are a couple of points to think about:

    CSS Variables Should Convey Purpose:

    Using meaningful names for CSS variables enhances code clarity, supports design flexibility, and improves collaboration by conveying purpose rather than just appearance. Names like --cyan don't indicate their use or adapt well to design changes. A context-based naming convention, such as --primary-background, is more intuitive and maintainable. This provides flexibility in Theming if you decide to change your design theme later, and the primary background color shifts from cyan to magenta, then the variable name --cyan no longer accurately represents its value, leading to potential confusion.

    I noticed an opportunity to slightly refactor the button styles and eliminate some redundancy:

    Redundant Styles: The background-color: var(--bright-yellow); in the button:hover selector is redundant since it is the same as the default button style. The transition property in the :hover state is also redundant, as transitions are typically defined on the base state and are automatically applied during the state change. So we can safely remove this.

    Transition Specification: While not strictly necessary, it's more efficient to specify which properties will transition rather than using transition: all 0.3s;. I've adjusted it to transition: background-color 0.3s, filter 0.3s; to target only the properties that change on hover.

    Original CSS

    button {
  color: var(--white);
  background-color: var(--bright-yellow);
  font-size: 1.5rem;
  font-weight: bold;
  letter-spacing: 0.5px;
  padding: 1.5rem;
  margin-bottom: 1rem;
  border: none;
  border-radius: 5px;
  transition: all 0.3s;
}

button:hover {
  cursor: pointer;
  background-color: var(--bright-yellow);
  filter: brightness(90%);
  transition: all 0.3s;
}

    Refactored To avoid redundency and to be more specific about the transition that takes place on a hover event

    button {
  color: var(--white);
  background-color: var(--bright-yellow);
  font-size: 1.5rem;
  font-weight: bold;
  letter-spacing: 0.5px;
  padding: 1.5rem;
  margin-bottom: 1rem;
  border: none;
  border-radius: 5px;
  transition: background-color 0.3s, filter 0.3s;
}

button:hover {
  cursor: pointer;
  filter: brightness(90%);
}

    These are crucial considerations. Admittedly, both in the past and during hurried moments now, I've occasionally overlooked these practices myself. Nonetheless, I wanted to share these insights with you, hoping to offer something valuable for your learning journey. It's unfortunate that there seems to be a current lack of advice or feedback on peoples solutions, but I'm here to support you in any way I can.

    Marked as helpful
    0
  • Jacksen•350
    @jacksen30Posted about 1 year ago

    Hello @Wael-Orraby, Good work getting the Javascript Functionality working well !

    I've got a small recommendation on the CSS, for a more consistent user experience, particularly regarding the resizing issue you're encountering with new quotes, consider the following modifications:

    .container Width Adjustment:

    On smaller screens, it might be beneficial to set the .container width to 90%. For larger screens, a fixed width, say 500px, can prevent the container from resizing every time a new quote is fetched. This ensures a consistent width across different quotes, enhancing user experience. However, keeping the height flexible is a good call, allowing it to adjust based on the quote's length.

    Body Height:

    Also I'd recommend setting the body height to 100vh. Currently, it's set to 96vh, but using the full viewport height ensures that the layout utilizes the entire visible screen area.

    I hope these suggestions enhance the responsiveness and appearance of your design. Let me know if you'd like any further clarifications or feedback.

    Marked as helpful
    1
  • olena-ostapenko•240
    @olena-ostapenkoSubmitted about 1 year ago

    I decided to add it to make the bone spin. while the library is loading.maybe there is a simpler solution?

    Responsive, API

    1
    Jacksen•350
    @jacksen30Posted about 1 year ago

    Hello @olena-ostapenko,

    Your solution looks great,

    A couple of small suggestions I have are, Instead of using an anchor tag for the button, it would be more semantically correct to use a button element. For instance:

    <button class="decor-circle" aria-label="Update Quote">
    <img class="dice" src="./images/icon-dice.svg" alt="">
</button>

    For the animation it could be simplified in the CSS like so:

    /* Defines the rotation animation */
@keyframes rotate {
  0% {
      transform: rotate(0deg);
  }
  100% {
      transform: rotate(360deg);
  }
}

    It might be beneficial to initiate the rotation at the beginning of your function and halt it upon either the successful retrieval of data or in the event of an error. This adjustment ensures the animation runs only while data is being fetched, rather than for a predetermined duration. This was the solution that I came to:

    function retriveQuote () {
    // Start the rotation animation
    button.style.animation = "rotate 1s linear infinite";

    fetch('https://api.adviceslip.com/advice')
        .then(response => {
            if (!response.ok) {
                throw new Error("Network response was not ok")
            }
            return response.json();
        })
        .then(data => {
            dynamicIdContainer.innerHTML = `Advice # ${data.slip.id}`;
            dynamicQuoteContainer.innerHTML = `&ldquo;${data.slip.advice}&rdquo;`;
            // Stop the rotation animation when data is received
            button.style.animation = 'none';
        })
        .catch(error => {
            console.log('There was a problem with the fetch operation:', error.message);
            // Stop the rotation animation upon a fetch error
            button.style.animation = 'none';
        });
    };

retriveQuote();

    I hope you find these suggestions helpful! Let me know if you have any questions or if there's anything else I can assist with.

    0
  • Jacksen•350
    @jacksen30Posted over 1 year ago

    Hello MTALAFA,

    Great work keeping up the momentum and submitting another challenge solution !

    I've just taken a quick look I think one of the main issues here is that the body doesn't have a height, so the total body height is only the height of the .container if you have a look in your dev tools and highlight the body element in the DOM / Elements you'll understand.

    Here's is a way that you could achieve what you are trying to:

    body {
    background-color: hsl(30, 38%, 92%);
    font-family: "Montserrat", sans-serif;
    
    /* ADDED THE BELOW PROPERTIES */
    height: 100vh;
    display: flex;
    justify-content: center;
    align-items: center; 
}
  
  .container {
    display: flex;
    
    /* REMOVED THE BELOW PROPERTIES */
    /* margin-top: 12rem; */
    /* justify-content: center; */
  }

    Hopefully this suggestion will help you enough to get unstuck with the centering of the main container, even if this isn't the solution that you decide on implementing.

    Marked as helpful
    0
  • Nikola•40
    @NikolaM4Submitted over 1 year ago

    This was harder challenge that the previouse. I learned how to better align div vertically, but I am not satisfied with my CSS so I will take a look to see how others did this challenge.

    3 Column Preview Card Component

    2
    Jacksen•350
    @jacksen30Posted over 1 year ago

    Congratulations @NikolaM4 on submitting your challenge solution, Considering this is only your second FE- mentor project you've done very well !

    After viewing your CSS I have some constructive advice on some improvements that could be made. I'd recommend researching these concepts and properties further to better understand them but I will give you the simple explanations and suggestions here, my recommendations don't cover all the possible fixes and refactors possible on your code but more the most impactful ones that will come in most useful to you on future challenges and web design projects.

    Border Radius for Cards:

    Current: CSS uses the border-top-right-radius and border-bottom-right-radius properties. Instead the border-radius shorthand property can be used this minimises the code required by setting values for all corners in a single line.

    This is the current:

    .orange-block {
  background-color: var(--orange-color);
  border-top-left-radius: 10px;
  border-bottom-left-radius: 10px;
}

.green-block {
  background-color: var(--green-color);
  border-top-right-radius: 10px;
  border-bottom-right-radius: 10px;
}

@media (max-width: 375px) {
 
  .orange-block {
    border-radius: 0;
    border-top-left-radius: 10px;
    border-top-right-radius: 10px;
  }

  .green-block {
    border-radius: 0;
    border-bottom-left-radius: 10px;
    border-bottom-right-radius: 10px;
  }

    This is the refactored version:

    .orange-block {
  background-color: var(--orange-color);
  border-radius: 10px 0 0 10px;
}

.green-block {
  background-color: var(--green-color);
  border-radius: 0 10px 10px 0;
}

@media (max-width: 375px) {
  
  .orange-block {
    border-radius: 10px 10px 0 0;
  }

  .green-block {
    border-radius: 0 0 10px 10px;
  }

    Button Hover Effect:

    Current: Separate hover effects are defined for .orange-block button:hover, .blue-block button:hover, .green-block button:hover. Suggestion: To streamline the CSS and prevent repetition, these can be removed. Instead, add a button:hover rule and use background-color: inherit, which will ensure the background color remains consistent upon hovering (seem the background-color required for each is the same as its parent containers background-color).

    This is the current:

    .orange-block button:hover {
  background-color: var(--orange-color);
  color: hsl(0, 0%, 95%);
}

.blue-block button:hover {
  background-color: var(--blue-color);
  color: hsl(0, 0%, 95%);
}

.green-block button:hover {
  background-color: var(--green-color);
  color: hsl(0, 0%, 95%);
}

    This is the refactored version:

    button:hover {
  background-color: inherit;
  color: hsl(0, 0%, 95%);
}

    Media Query Breakpoint:

    Current: The media query breakpoint is set at 375px. Suggestion: Consider increasing the breakpoint to somewhere around 700px. While the design indicates a mobile view at 375px width, that doesn't dictate the optimal breakpoint. Adjusting this can provide a more consistent user experience across varying screen sizes.

    Below I've commented within the code some properties that have been incorrectly used and can be removed, and some new ones that I have added to help with the overall layout as well as the responsiveness on the page:

    * {
  margin: 0;
  padding: 0;
  box-sizing: border-box;  /* ADDED - When using border-box, the width and height you set for an element will include its padding and border. This makes it simpler to create layouts that maintain a consistent size regardless of the padding or border applied to the element.*/
}

body {
  background-color: hsl(0, 0%, 95%);
  font-family: "Lexend Deca", sans-serif;
  /* position: relative; */  /* REMOVED - Position not required in this context */
  font-size: 15px;
  height: 100vh;

/* ADDED ALL BELOW PROPERTIES - Using these flex properties will help to center the main element thats nested within the body element  */

  display: flex;
  flex-direction: column;
  justify-content: center;
  align-items: center;
}

main {
  /* position: absolute; */  /* REMOVED - Position not required in this context */
  max-width: 850px;
  display: flex;

  
/* REMOVED ALL BELOW PROPERTIES - These were used with your position property that has now been removed  */
  /* margin: 0 auto; */
  /* top: 50%;
  left: 50%;
  transform: translate(-50%, -50%); */
}

.block {
  /* height: 360px; */  /* REMOVED, setting a fixed height is causing the element to break at smaller screen widths, try to limit setting fixed heights and widths were possible */
  padding: 40px 40px;
}

/* CHANGED max-width: 375px to 700px for better responsive layout */
@media (max-width: 700px) {
  main {
    margin: 50px 0;
    flex-direction: column;
    width: 90vw;

    /* REMOVED ALL BELOW PROPERTIES - These were used with your position property that has now been removed  */

    /* top: 100%;
    left: 50%;
    transform: translate(-50%, -70%); */
  }

  .block {
    /* height: 300px; */ /* REMOVED, setting a fixed height is causing the element to break at smaller screen widths, try to limit setting fixed heights and widths were possible */
    padding: 40px 50px;
  }
}

    I hope you have found my recommendations helpful.

    Keep up the great work of learning HTML and CSS, Looking forward to seeing you grow and improve as a front-end web developer and submit many more challenge solutions within this community !

    1
  • Mtalafa•350
    @MtalafaSubmitted over 1 year ago

    Hi,

    I`m new here and I am new to web development. This is my first challenge. My only question is: Is there a better way to do it? Any feedback is more than welcome.

    3 column preview card using flexbox

    1
    Jacksen•350
    @jacksen30Posted over 1 year ago

    Hello MTALAFA,

    Great Effort on your first Frontend Mentor Project, The first few are always the hardest when first starting to learn.

    I've had a look at your CSS and can see some easy changes and refactoring we can do to improve it. I'd recommend looking up these concepts and properties further on MDN for a better understanding but I will give you the simple explanations and suggestions here (this isn't a complete solution but it will go a long way to improving it and helping you on your next challenge) :

    CSS Updates & Refactoring

    Viewport Height on Body:

    Current: The height: 100vh; property is set inside the .container class. Suggestion: Relocate this property to the body element selector. This ensures that the entire viewport height is consumed by the body, providing a consistent base for all internal elements.

    Button Hover Effect:

    Current: Separate hover effects are defined for btn-1:hover, btn-2:hover, and btn-3:hover. Suggestion: To streamline the CSS and prevent repetition, these can be removed. Instead, enhance the existing .btn:hover rule by adding background-color: inherit, which will ensure the background color remains consistent upon hovering.

    Border Radius for Cards:

    Current: The cards / overall component doesn't have rounded corners like the design files. Suggestion: Implement the border-radius shorthand property within the individual card classes. This not only simplifies the code but also sets values for all corners in a single line.

    For larger screens:

    .sedans {
  border-radius: 7px 0 0 7px;
}

.luxury {
  border-radius: 0 7px 7px 0;
}

    For smaller screens (in the media query):

    .sedans {
  border-radius: 7px 7px 0 0;
}

.luxury {
  border-radius: 0 0 7px 7px;
}

    Media Query Breakpoint:

    Current: The media query breakpoint is set at 375px. Suggestion: Consider increasing the breakpoint to 500px or more. While the design indicates a mobile view at 375px width, that doesn't dictate the optimal breakpoint. Adjusting this can provide a more consistent user experience across varying screen sizes.

    Responsive Design Adjustments:

    Current: The design lacks optimal responsiveness, especially for mobile screens. Suggestion: To enhance mobile compatibility:

    Update the .container within the media query to employ flex-direction: column and adjust its width to 95% for better spacing.

    Remove the max-width constraint on .car-choice by setting it to unset. This grants the content more flexibility to adapt to diverse screen widths.

    I hope my recommendations will be beneficial for your current and future projects. Keep up the learning and I look forward to seeing more of your projects in the future

    Marked as helpful
    0
  • JessicaSamtani•160
    @JessicaSamtaniSubmitted over 1 year ago

    Second project down - scratched my head at first with regards to the positioning of "button" as it is not inline with the other elements within the box. Happy to hear suggestions on what could be better on my code. Working on lots more. :)

    3 Column Preview Card _ Front End Mentor Challenge

    3
    Jacksen•350
    @jacksen30Posted over 1 year ago

    Feedback on Your Solution

    Firstly, congratulations on getting your design to visually match the requirements the live site looks great.

    Here's a few quick and easy to implement code fixes / improvements

    Semantic HTML Improvements:

    Main Tag: For enhanced semantic clarity, consider enclosing the primary content (typically everything outside of the header and footer) within the <main></main> tags.

    Header Consistency: Aim for a single <h1> tag per page to maintain hierarchical clarity and improve SEO.

    Refactored CSS Recommendations:

    Consolidate Common Styles: Rather than applying individual CSS rules to each of the .sedans, .suvs, and .luxury classes, it's more efficient to group and apply shared styles collectively. For example:

    .sedans,
.suvs,
.luxury {
        display: flex;
        flex-direction: column;
        justify-content: center;
        align-items: center;
        margin: 30px 0;
        width: 250px;
    }

    Unified Button Hover Styles: I added a extra class btn to all the buttons elements in your HTML. By consolidating hover styles into a singular rule and using the background-color: inherit property, you can minimize redundancy and ensure consistent behavior. Notably, this adjustment will correct the color inconsistency present in .button-2:hover.

        .btn:hover {
        background-color: inherit;
        color: white;
        border: 2px solid white;
    }

    Optimized Border Radius Syntax: Utilize the shorthand border-radius property to simultaneously set values for all four corners, promoting cleaner and more concise code.

    .sedans {
        border-radius: 7px 7px 0 0;
}

    Here is all the updates I've made - more could be done but I think if you just compare it to the original and research the changes made from the original it'll greatly improve your future challenge solutions, I hope this helps you a lot

    * {
    padding: 0;
    margin: 0;
}

body {
    background-color: white;
}

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

h1 {
    font-family: 'Big Shoulders Display', cursive;
    color: hsl(0, 0%, 95%);
    align-self: first baseline;
    margin-left: 35px;
    margin-top: 30px;
}

p {
    font-family: 'Lexend Deca', sans-serif;
    font-size: 15px;
    color: hsla(0, 0%, 100%, 0.75);
    margin: 35px;
    padding-bottom: 20px;
}

.sedans,
.suvs,
.luxury {
    display: flex;
    flex-direction: column;
    justify-content: center;
    align-items: center;
    margin: 30px 0px 0px 0px;
    width: 250px;
}

.sedans {
    background-color: hsl(31, 77%, 52%);
    border-radius: 6px 0 0 6px;
}

.suvs {
    background-color: hsl(184, 100%, 22%);
}

.luxury {
    background-color: hsl(179, 100%, 13%);
    border-radius: 0 6px 6px 0
}

button {
    width: 150px;
    height: 40px;
    border-radius: 19px;
    border-style: none;
    font-family: 'Lexend Deca', sans-serif;
    color: black;
    align-self: baseline;
    margin-top: 20px;
    margin-bottom: 40px;
    cursor: pointer;
}

.button-1 {
    color:hsl(31, 77%, 52%);
}

.button-2 {
    color: hsl(184, 100%, 22%);
}

.button-3 {
    color: hsl(179, 100%, 13%);
}

.btn:hover {
    background-color:inherit;
    color: white;
    border: 2px solid white;
    border-color: white ;
}

.sedan-icon, .suv-icon, .luxury-icon {
    display: block;
    align-self: first baseline;
    margin-top: 40px;
    margin-left: 25px;
}

@media only screen and (max-width:750px) {
    

    .wrapper {
        display: flex;
        flex-direction: column;
        height: 100%;
    }

    .sedans, .suvs, .luxury {
        height: 60vh;
        padding: 5px;
        margin-bottom: -30px;
        padding-bottom: 10px;
    }
    
    .sedans {
        border-radius: 7px 7px 0 0;
    }

    .luxury {
        border-radius: 0 0 7px 7px;
    }
}

    Keep completing challenges and receiving feedback and you'll be amazed at how much you will learn in a short time. I'm looking forward to seeing many more projects from you in the future. Happy Coding !

    Marked as helpful
    0
  • Samriddh Singh•100
    @saketbyteSubmitted over 1 year ago

    Again learning bit by bit and practising while reading blogs, asking on StackOverflow and of course YouTube video tutorials to understand how things work. Hosted on Netlify. I am also learning Github side by side, by maintaining each project in a different branch of the same repository to remove clutter of several repositories, and hosting from the branches itself. * that supercool feeling you get when you use bash *

    Signup-login-form

    2
    Jacksen•350
    @jacksen30Posted over 1 year ago

    Hi @saketbyte,

    I'll start by saying great effort, it looks very similar to the design and it is a tricky project when starting out. I've just completed it myself and learnt a lot by completing it.

    I've got a few suggestions regarding best practices ect.

    1. You should check your HTML for correct indentation, especially line 25 -29

    2. All input fields should have an accompanying <label> (for accessibility) for example

    <label for="firstname" class="sr-only">First Name</label>
<input type="text" name="fname" id="firstname" placeholder="First Name" value="">

    The <label for=""> attribute should be equal to the id="" of the <input> it is related to.

    note the sr-only class on the label - see the css you can use below. This allows you to visually hide the label text for the form but It will still be read by screen reader technology for the visually impaired

    .sr-only {
    position:absolute;
    left:-10000px;
    top:auto;
    width:1px;
    height:1px;
    overflow:hidden;
    }

    3. You need to have a closer look at your Javascript as after the form has been submitted with inputs that did not pass the validation the error text such as "First name is required is not removed once a correct First name is entered and submitted

    4. Consider Implementing a css reset to ensure that different browsers will render your code in the same way.

    This can be done in a reset.css file and used with every website you build.

    You can read more about this at Andy Bells CSS Reset

    Please mark my answer as helpful if it has helped you in any way

    Keep up the great work ! I look forward to seeing more solution submission from you in the future.

    Marked as helpful
    1
  • prchristie•50
    @prchristieSubmitted over 1 year ago

    Theres a weird issue when I reduce the screen size to very small horizontally the background (body and main elements) stop taking up 100% of the screen even though I have them both set to 100% height. I've tried setting width to 100% as well but it didn't do anything.

    Solution using plain HTML and CSS

    1
    Jacksen•350
    @jacksen30Posted over 1 year ago

    @prchristie great work on this challenge it looks amazing !

    As for your weird responsive resizing issue with background ect, I can't explain why it happens but it's a common thing I've found. I've check your site in my browser and from what I can see it only happens at sizes that are so small / narrow you wouldn't encounter a screen that small in real world usage, so I would not stress at all on this!

    A couple of bits of constructive criticism I have is:

    1- My thinking on a component of this size is that so many css files are unnecessary and actually make it harder to read what is happening, could all be combined in to one... just my personal opinion.

    2- I didn't notice any css reset. Its always good to use a css reset as the first selector in your stylesheet such as something basic like this:

    * {
    padding: 0px;
    margin: 0px;
    box-sizing: border-box;
}

    or for something more advanced you could look at this Josh W's Css Reset

    Different browsers use different default margins, causing sites to look different by margins. The * means "all elements" (a universal selector), so we are setting all elements to have zero margins, and zero padding, thus making them look the same in all browsers.

    Please mark this answer as helpful if it is, Happy coding

    0