Skip to content
OverviewSolutions
16
Comments
10
JK
@JakubKepak

All comments

  • Radomir Mijovic•150
    @radomir-mijovicSubmitted almost 3 years ago

    I have finished Product Feedback Guru challenge. It's using React on front-end and Django on back-end. I would love for you to test the app. In order to add-update-delete new feedback, post comment or reply you need to create an account. You do not have to use real names or pictures but they are mandatory if you want to create a account. Also please do not delete feedback that you did not post yourself because it took ma a while to fill out the database.

    Using React for frontend and Django Rest Framework for backend

    2
    JK•345
    @JakubKepakPosted almost 3 years ago

    Very nice job Radomir! I've noticed a small thing which might take your solution even higher, in my opinion :).

    Sometimes you use under score in variables names. For instance, up_vote in In ButtonUpVote.js. I would stick with camelCase only, it's more consistent.

    Cheers!

    0
  • JK•345
    @JakubKepakPosted over 3 years ago

    I like your solution! very nice 👏

    1
  • JK•345
    @JakubKepakPosted almost 4 years ago

    Hi Mohak, your solution is awesome. 👏👌

    I would maybe add max-width to keep it nice on larger screens.

    Keep it up! Rally like it! 🙌

    0
  • Louis Terryn•40
    @LouisterrynSubmitted almost 4 years ago

    Hello everyone, I've finished the challenge and I think that everything works perfectly.

    If you have any tips about how to optimize the code do not hesitate to tell me !

    Have a great day

    Article component

    1
    JK•345
    @JakubKepakPosted almost 4 years ago

    Hi Louis, very nice work! 👌👏

    In your code, you use px for size. I would maybe use some relative measure like rem or em. Also, I would fill in alt attributes for accessibility as the report suggests.

    Anyway, I really like your solution. Keep it up! 🙌

    0
  • JK•345
    @JakubKepakPosted almost 4 years ago

    Hi Chetan! very nice solution! 👏

    I would maybe consider adding media query on about 375px and reorder the columns.

    Anyway, good job!

    0
  • JK•345
    @JakubKepakPosted almost 4 years ago

    Hi Chris-Ann C. ! Nice work 👏

    To centre all content to the centre of a page a would set display:flex to the body and then add justify-content: centre and align-items: centre. In order to work, you would also need to set the property height on the body element. For instance height:100vh. Also, I would maybe set max-width proepty to .wrapper in order not to fill all space in wide screens.

    Anyway, nice job! Keep improving 🙌

    1
  • JK•345
    @JakubKepakPosted almost 4 years ago

    Hi Lpz, nice job! you have it almost pixel perfect! 👌👏

    I would maybe change position of the image with object-fit property be more like the design (https://css-tricks.com/almanac/properties/o/object-fit/)

    anyway, nice work! Keep improving! 🙌

    1
  • InduRajput•120
    @InduRajputSubmitted almost 4 years ago

    Please do review my code and give your suggestions. With many trial and error i some how managed to get the design structure but still very much confused on the display of different sections. Any kind of help is appreciated. Thanks in advance :)

    Social Proofing section using dispaly positioning.

    1
    JK•345
    @JakubKepakPosted almost 4 years ago

    Hi InduRajput, nice work! The desktop view is pretty close to the design. 👏

    My food for thoughts:

    • I would name class attributes in HTML something meaningful. That is just my opinion. It's easier to navigate in css file
    • I would position the boxes ( sectionX in your code) with css grid or flexbox. I would create a container for all with display: flex or grid. You can then shift them by adding margin.
    • Similarly, I would approach the rating-sec container.

    With this you can then change ordering by changing display property on container to flex-direction: column, if you would use flexbox.

    Keep coding! 🙌

    0
  • JK•345
    @JakubKepakPosted almost 4 years ago

    Hi Michelle, nice work! I would maybe add media query In about 1000px and perhaps change the layout to two columns or smaller the font-size.

    0
  • JK•345
    @JakubKepakPosted almost 4 years ago

    Man, that is one awesome solution! One thing I would maybe do differently is to set outline to none when .person_article-share is focused and I would add .person_article-share:focus same as you have .person_article-share:hover.

    But that's maybe just my view on outlines.

    Really nice job!

    1