Skip to content
OverviewSolutions
26
Comments
6
Avatarfreak
@avatarfreak

All comments

  • Avatarfreak•655
    @avatarfreakPosted about 3 years ago

    Hey, nice transition on the toggle button.

    • For toggling, I prefer using the radio button. It is easy to target with js.
    • I did the same kind of transition with a different approach. Have a look at my solution . Feedback would be welcomed. Happy coding.
    Marked as helpful
    0
  • Avatarfreak•655
    @avatarfreakPosted about 3 years ago

    Nice and congratulation on completing the challenge but here is some issue that needs to be rectified.

    • your site is not published. I'm guessing you may have entered the wrong path in the live URL field.
    • Correct path to your solution solution
    • In the readme file. You gave the wrong path. 404 pages are showing.
    • Image is missing.

    Happy coding. These are minor issues and can easily be fixed it.

    Marked as helpful
    0
  • Avatarfreak•655
    @avatarfreakPosted about 3 years ago

    It is really looking nice. One minor issue in mobile devices, there is a horizontal shift in layout, overflow kind of issue. Happy coding.

    1
  • kevin•415
    @imonaarSubmitted about 3 years ago

    This looked ease at first but challenged me a lot. i had to restart over at some pointswhen everything wasnt working. my major concern is the buttons. As you can notice my button layout breaks when you adjust for different mobile screens.

    1. how did you approach approach laying out the buttons? did you use buttons for each indivual page? or just a single pair of buttons absolutely positioned like i did?

    2. i haven't touched on accessibility yet because i need to solve that button layout that is bugging me

    3. more comments and corrections are welcome

    sass. parcel, netlify. coding-bootcamp-testimonials

    1
    Avatarfreak•655
    @avatarfreakPosted about 3 years ago
    • Good work, Design-wise it is good and responsive.
    • For the button try using pseudo-element.
      ::after or ::before.
    • In my opinion, it is easier to target elements. As it will always be relative to its immediate parent. something like this example.
    .image-container{
    position:relative;
}

.image-container::after{
    content:'';
    position:absolute;
    top:0;
    left:0;
    width:100%;
    height:100%
}
    Marked as helpful
    0
  • Avatarfreak•655
    @avatarfreakPosted about 3 years ago
    • Design is good and responsive.
    • I think your email validation is not working properly after entering the correct email. It still complains wrong email.
    1
  • Avatarfreak•655
    @avatarfreakPosted about 3 years ago

    Very well done from a design perspective.

    One thing I found a little unusual is transition duration. I meant to say there is a delay of few seconds during the transition.

    Secondly, you could have made a single component and target the attribute and replace image and quotation using javascript

    element.setAttribute('src', 'path/to/image/')
element.textContent= "quotation"

    For slide show logic you can check my link given below. It is simple javascript.

    slideshow

    0