Skip to content
OverviewSolutions
7
Comments
10
Gregório Francisco
@gregoriofrancisco99

All comments

  • Gregório Francisco•235
    @gregoriofrancisco99Posted over 1 year ago

    Have to agree that this one was a funny one to complete. 😊

    1
  • Joanna Skrzypczak•530
    @joaskrSubmitted almost 2 years ago

    Hi community, Second challenge today - finished. I'm quite proud of myself. I wanted to do the challenge in less than hour and I accomplished it.

    Process

    • I only used HTML, CSS and vanilla js for this challenge. I used mobile-first approach for styling.
    • I fetch advices from Advice Slip JSON API

    Advice Generator - vanilla js

    1
    Gregório Francisco•235
    @gregoriofrancisco99Posted almost 2 years ago

    Hello, Joana. Your solution looks amazing. Didn’t have the time to go for this challenge yet, but you just inspired me. 😅 Okay, let’s get to business, now.

    Took a look at your html, and I think you should switch the h1 and p tags one for each other. Because as far as I can understand, headings are, for the user, some kind of directions of what kind of content he can find in a certain section, and that should be short and explanatory as “Advice #117”. 😊

    This my only suggestion. Loved your solution. ❤️ And remember... keep coding 💻

    Marked as helpful
    0
  • Gregório Francisco•235
    @gregoriofrancisco99Posted almost 2 years ago

    Hello, Liya. Congratulations for your submission 🥂 Your design looks awesome, but there’s a little room for improvement. ☺️

    Took a look at your html, and one of the best practices is to use semantic and more meaningful tags instead of using div, a generic and low meaning tag, everywhere. This article should help you getting started. 👌

    In total there are six type of headings, and h2 is a level two heading which you can have as many time as you need. This article should help.

    Noticed you had a hard time trying to center the content. Well, the following code should help

    body {
width: 100%;
height: 100vh;
}
#card {
Width: 910px; /* for example*/
Margin: auto;
}

    Other good solution would be to use the flexbox property.

    Well, hope I was able to help! And remember... keep coding 💻

    Marked as helpful
    0
  • Gregório Francisco•235
    @gregoriofrancisco99Posted almost 2 years ago

    Hello, noran! Your solution looks good. Congratulations on your submission🥂

    Just to make it better, you should try to replace the div tags you used to wrap the content of the card by a section tag. Because div tag is generic and it don't mean too much to the browser.

    This is my suggestion for this solution, and remember... keep coding 💻

    0
  • Mohamed Lajnef•60
    @momolajnef1Submitted almost 2 years ago

    I loved working on this project. This is the best css code I have written so far. I tried my best to make a good Semantic HTML Structure. Is there anything that I need to improve?

    3 column preview card component

    1
    Gregório Francisco•235
    @gregoriofrancisco99Posted almost 2 years ago

    Greetings, pal! You solution looks really fine, even on mobile. Although there’s a little points where we could improve.

    Saw your html, and If it was me, I shouldn’t had a div to contain everything, while I already have the main tag.

    The h1 tag is a level one heading and there should be only one per page. I noticed you have a couple ones in your code. Because you’re using a section tag, it should be preferable to change it for a h2, because section tags are usually followed by level two heading, a.k.a. h2. You can read more about it HERE.

    To hold the icons, one of the best practices should be to link them to your document using a img tag, instead of an empty div.

    Finally, I would strongly recommend to use an a tag, instead of button. Because, on my way of thinking, the page should take the user to other page, or other panel.

    Well, those are my suggestion. Hope I was able to help. And remember... Keep coding 💻

    0
  • Reza Jaber•200
    @rezajaberSubmitted almost 2 years ago

    Hey! I am Reza 🙂 I just recently started to learn web-development and would like to get every little help out there. So if you have the time, feel free to look over my project/projects and give me some advice to get better as fast as possible.

    Project #10: Some questions for this "Single-Price-Grid" project:

    1. Did I center everything right, or was there a easier way?
    2. Was there a easier way to design some things in way less code?
    3. What did I do completely wrong, and just have luck within this project?
    4. What would you do different?

    I appreciate it✌️

    Single price grid component

    1
    Gregório Francisco•235
    @gregoriofrancisco99Posted almost 2 years ago

    Hello, Reza! Gotta tell you, loved your solution. And the comparison screenshot speaks by itself! Congratulations! Even tough the design looks really good, I took a look at your code and I will raise some points.

    You should try to wrap all your main content inside a <main>.

    And if you have more than one <section> you should consider wrapping them inside an <article>.

    <h1> are the top level heading in a page, and there should be only one of it. and <h2> is what follows it on the heading hierarchy. Clique here to get to know them all (from <h1> to <h6>) and what are they meant for.

    And I saw you used a <button> for the sign up. And it's not really wrong, but for me, I think you should use an <a>. Because it seems it will take the user somewhere else, or open some other content.

    I saw you used an <ul> for the last card. That was awesome!!! 🎉

    Well, those are my comments for this solution. And remember... Keep coding 💻

    Marked as helpful
    0
  • Gregório Francisco•235
    @gregoriofrancisco99Posted almost 2 years ago

    Hey, pal! Your solution looks stunning, congratulations 🎉. Although there's a little space for improvement.

    Saw that you set html, body{height: 100%;}. Well, this is not really wrong, but also this is not something we do often. And because of this, on mobile devices, it's impossible to see your solution completely. But you can solve it applying the following settings to your stylesheet:

    html, {
height: auto; /* Or just remove it 😅*/
}

body {
height: 100vh;
}

    Noticed that you have two height properties conflicting in you main tag style. Resolving that should make your .css happy 😊. And if you already have a main tag encapsulating everything, I would advice you to remove the div class="container".

    Well those are my suggestions.

    And remember... Keep coding 💻

    Marked as helpful
    1
  • Gregório Francisco•235
    @gregoriofrancisco99Posted almost 2 years ago

    Greetings, dear Dany, hope you're okay. Saw your solution and I've to say, I like the innovations you added to it. It gave the challenge a self look.

    Although, there's a few points that should make to solution way better than it's right now.

    • Header:
    1. You create a hard situation for yourself by creating a lot of sub-levels just to place the Mark all as read button. You should try to make your HTML document as clean as possible for yourself and for anyone reviewing or upgrading your code.
    2. You used an unordered list in a situation where it's not really need. You should solve it with a simple <a href=#>Mark all as read</a>. Learn more about <a>.

    And remember... Keep coding 💻

    Marked as helpful
    0
  • Gautam Juyal•330
    @gautamjuyalSubmitted almost 2 years ago

    First project with react. I am still familiarizing with the ecosystem. Please do tell the best practices and things that could be improved upon. Thank you.

    Notifications page using ReactJS

    2
    Gregório Francisco•235
    @gregoriofrancisco99Posted almost 2 years ago

    Hey, pal. Congrats on the submission.

    Your solution is very impressing nevertheless, there are some points I think you should try to improve.

    • HTML Semantic: instead of using div for 'everything' - which doesn't mean to much to the browser - you should try to use more meaningful tags. Don't hesitate on Googling you don't know them. 😄

    • The <strong> is not meant to make the word bold just for visualization purposes. Click HERE to get to know him better

    • The alt property in the <img> plays a big role when, for some reason, the image cannot load, without mentioning that people with disabilities rely completely on it to know what is the image content. So you should try to make it the more explanatory as possible.

    • To create the red dot on the unread notifications, saw that you used a span with a dot on it. For this purpose I'd use a pseudo element. You can learn more about it here

    Hope I was able to help! And remember Keep coding... 💻

    Marked as helpful
    0
  • Gregório Francisco•235
    @gregoriofrancisco99Posted over 3 years ago
    This comment was deleted almost 2 years ago
    1