Skip to content
OverviewSolutions
62
Comments
82
Byron
@byronbyron

All comments

  • Byron•2,290
    @byronbyron

    @DanK1368 Hia mate.

    Try position: fixed; instead of position: absolute;, to fix the scrolling issue.

    @media screen and (max-width: 670px) {
    .ggprRt {
        /* position: absolute; */
        position: fixed;
        top: 0px;
        left: 0px;
        height: 100vh;
        background-color: rgba(0, 0, 0, 0.5);
        transform: translateX(110%);
        opacity: 0;
    }
}
    Marked as helpful
  • Byron•2,290
    @byronbyron

    Hi @Chillidot

    If you remove the height: auto; from the your style.css below, that should fix it in safari!

    /** Circle **/
svg{
    max-width: 100%;
    /* height: auto; */
}

    Hope that helps!

    Marked as helpful
  • Byron•2,290
    @byronbyron

    Hi @ujwalvinay

    You can put the overlay hover on the image by adding an image wrapper and using a pseudo element, something like:

    <div class="img-wrapper">
    <img src="images/image-equilibrium.jpg" alt="image" id="banner">
</div>

    .img-wrapper {
    position: relative;
}

.img-wrapper::before {
    content: '';
    background: url('../images/icon-view.svg') center no-repeat hsla(178, 100%, 50%, 0.5);
    display: block;
    border-radius: 1.2rem;
    position: absolute;
    top: 0;
    right: 5%;
    bottom: 0;
    left: 5%;
    z-index: 100;
    visibility: hidden;
    opacity: 0;
    transition: visibility 0.3s, opacity 0.3s;
}

.img-wrapper:hover::before {
    visibility: visible;
    opacity: 1;
}

    Hope that helps! 👍

    Marked as helpful
  • Byron•2,290
    @byronbyron

    Hi @MikeAngel2

    You should be able to center the second card by updating your index.js to set display flex instead of block.

    $(".Thanks").css('display','flex');

    This bit of CSS should help to center the text on the second card as well

    .Thanks {
    align-items: center;
    text-align: center;
}

    Hope that helps!

    Marked as helpful
  • Byron•2,290
    @byronbyron

    @Abrosss Looks good!

    Your question is probably one for whoever designed it. I went with a fixed width, but I don't think it matters too much.

    Few html/accessibility issues:

    Your <section class="page"></section> element is probably better off being a <main class="page"></main> (Document should have one main landmark)

    Also need a <h1> element somewhere inside your <section class="container"> (Page should contain a level-one heading)

    Marked as helpful
  • Byron•2,290
    @byronbyron

    @Ganesh-Rathor To style range input on chrome/brave (webkit) you can use:

    input[type=range]::-webkit-slider-runnable-track {}
input[type=range]::-webkit-slider-thumb {}
    Marked as helpful
  • Byron•2,290
    @byronbyron

    Hey @jillpandya2594, Hopefully this fixes your grid!

    .container {
    margin: auto;
    max-width: 40rem;
    padding: 2em;
    display: grid;
    grid-template-rows: 1fr 1fr;
    grid-template-columns: 1fr 1fr;
}

main {
    padding: 2em;
    background-color: #fff;
    grid-column: span 2;
    align-self: flex-end;
}

section {
    display: flex;
    flex-direction: column;
    padding: 2em;
    background-color: hsl(179, 62%, 43%);
}

aside {
    padding: 2em;
    background-color: hsl(180, 62%, 47%);
    line-height: 1.6;
}
  • Byron•2,290
    @byronbyron

    Nice job @NellyisDevv

    I would avoid using the <center> tag as it's well out of date, and no longer recommended.

    To display your QR code a tiny bit better on really small screen widths (around 320px) I would do something like this:

    .qr-container-background {
    background-color: var(--White-);
    padding: 20px;
    border-radius: 20px;
    max-width: 320px; /* Set max width of card */
    width: 100%; /* Full width up until 320px */
}

.qr-container-photo img {
    border-radius: 10px;
    width: 100%; /* Set image to fill width of the container */
    height: auto;
}

    For a small project like this, I think your CSS file set up is absolutely fine!

    Also, you've got a few Accessibility/Html issues that you could resolve for a perfect score.

    Marked as helpful
  • Byron•2,290
    @byronbyron

    Hey @TotallySly, it looks alright!

    For the button hover, I used a slight opacity instead of the background colour, something like:

    .payment--container button:hover,
.payment--container button:focus {
    opacity: 0.75;
}

    For the sizing thing, it depends on how picky your client is in the real world 😅. When I want to get the image size from the design image, I usually open it up in Preview (mac) and drag a square around the image to get the dimensions.

    For your Order Component, it looks like your background images aren't coming through. Something like this should help I think (feel free to play around with the background-size values):

    body {
    ...
    background: url(images/pattern-background-mobile.svg) no-repeat, top center hsl(225, 100%, 94%);
    background-size: 100% 194px;
}

@media screen and (min-width: 908px) {
    body {
        background-image: url(images/pattern-background-desktop.svg);
        background-size: contain;
    }
    Marked as helpful
  • Byron•2,290
    @byronbyron

    @EmmanuelOloke Looks good!

    Your <section> elements ideally need a heading tag inside them somewhere 👍

    That's my 5 pence 😅

    Marked as helpful
  • Byron•2,290
    @byronbyron

    @DanK1368 Looks good!

    1. background-image: url(../../assets/icon-check.svg); will get you the checkbox!
    2. It sounds like your todo ids aren't correctly being set. I had a similar problem and solved it by using SortableJS and nanoid
    Marked as helpful
  • Byron•2,290
    @byronbyron

    @AdamMzkr Looks great!

    Just missing a <main> tag, that's all.

    Nice job 👍

    Marked as helpful
  • Byron•2,290
    @byronbyron

    Looks good @jackzorola10

    The landmarks issue basically means you're missing a <main> tag. Try wrapping everything inside the <body> with a <main> tag.

    Hope that solves your issue 👍

    Marked as helpful
  • Byron•2,290
    @byronbyron

    Hey @dreamspice your BEM classes don’t look too bad at all! The main point is that you’ve picked a convention and stuck to it. 👍

    Marked as helpful
  • Byron•2,290
    @byronbyron

    Nice work, @dinesh671

  • Byron•2,290
    @byronbyron

    Not bad @Christ-Kevin

    I'd suggest having a look at the accessibility/html issues flagged by the FE mentor report 👍

    • All page content should be contained by landmarks
    • Bad value auto for attribute height on element img: Expected a digit but saw auto instead.
    • Section lacks heading. Consider using h2-h6 elements to add identifying headings to all sections.
    • etc.

    Other than that looks good! The massive height is a new one as well 😆

  • Byron•2,290
    @byronbyron

    Hi @Reandyx

    Looks really good!

    Few accessibility issues flagged the FE Mentor report:

    • Heading levels should only increase by one
    • Document should have one main landmark

    Both should be fairly easy to fix by making sure your heading start at <h1> and continue in order. The second issue can be fixed by wrapping all the content in a <main> tag.

    Your transitions when the theme switches are brilliant!

  • Byron•2,290
    @byronbyron

    Hi @Ashraf-7

    Looks like you've got a few issues flagged by FE Mentor's report thingy:

    • Buttons must have discernible text
    • Section lacks heading. Consider using h2-h6 elements to add identifying headings to all sections.

    Other than those, it looks good 👍

    Marked as helpful