Skip to content
OverviewSolutions
6
Comments
13
dgjenni2
@dgjenni2

All comments

  • Gabriel Alves•50
    @Gabriel-Alves-95Submitted about 2 years ago

    Hello, everyone!

    This is my solution to the Advice generator app challenge. I have used HTML, CSS and Vanilla JS only.

    I am new to fetch so I would appreciate any suggestions or remarks about its use in my project. Specially, I would like to hear from you if there is a better way of employing the spinner than the one I have made. If you simulate a slow 3G connection in your browser, you will see that the spinner does not work properly.

    Thanks in advance.

    Advice generator app with Vanilla JS

    1
    dgjenni2•320
    @dgjenni2Posted about 2 years ago

    It looks like the loading spinner loads correctly after the first load when it does not.

    An idea I had to fix that behavior was rather than replacing the entirety of the html during the load, maybe you could try making the spinner a hidden element absolutely positioned in the center of the screen. Then, when the button is clicked, toggle the css that hides the spin to make it appear, and when the fetch call gets a response, then make the spinner hide again. Just an idea, haven't had the time to try out the fix to guarantee it'll work, but conceptually I don't see why that wouldn't fix the problem by having the image element load in sooner.

    Marked as helpful
    0
  • dgjenni2•320
    @dgjenni2Posted about 2 years ago
    1. Looks like you keep the desktop photo displayed in both mobile and desktop layouts. It would probably look a lot better on mobile if you swap out with the mobile image with your media query breakpoints.
    2. I'd increase the margins on the top and bottom of most elements and increase the text size a bit. Overall, the solution seems cramped compared to the original design.
    3. If you can, avoid hardcoding heights of elements as that can create weird behaviors for different screensizes.
    0
  • NKWETAKEM TABO BRUNO•490
    @BLADEHEDASubmitted about 2 years ago

    This challenge was very amazing since u had alot to learn through it . Thoughi did not ise grids as intened to , i managed to use flexboc/ Anyone with notion on how to import locally a json folder into javascript code . Any feedback would be highly appreciated .

    time-tracking-dashboard

    1
    dgjenni2•320
    @dgjenni2Posted about 2 years ago

    To import the JSON within a JQuery file, you can use this method that I've used in another project:

    // Use Jquery getJSON method, which takes two parameters... path to JSON file and a callback function to process the JSON once it is loaded $.getJSON("./change/this/to/path/to/file", function (json) { callBackFunction(json) })

    function callBackFunction(jsonData) { // Do something with the data here }

    https://stackoverflow.com/questions/7346563/loading-local-json-file (Also has some other potential options you might want to try)

    Marked as helpful
    0
  • dgjenni2•320
    @dgjenni2Posted about 2 years ago
    1. On the desktop view, I would increase the margins so the content is a little more centered.
    2. I don't like how the icons are positioned relative to the screen; it creates a weird effect where the buttons slide across the width of the text area and then exceed it depending on the sceensize. Could include them in the text container so they align with maximum size of the text content?
    3. You might want to readjust the media query width that transitions from mobile to desktop. There's a good 300ish pixel range from about 640-940 where all or some of the text gets pushed off screen. The mobile-first approach should technically allow you to use the mobile layout at any size, so if you change that width to change when the image and text fit on the screen, it would improve the layout dramatically in my opinion.
    4. You might want to add a max-width to the main content area for when displaying on extra large monitors.

    Overall, I really like your submission. It's very clean. It makes good use of additional html5 elements besides divs. It also declares the color scheme in css variables. Good work. Just needs some finishing touches.

    Marked as helpful
    0
  • dgjenni2•320
    @dgjenni2Posted about 2 years ago
    1. I like how your class names are specific to sections, but you could make some css classes shared between elements, such as using a container class in each section to set the margin and padding of the text areas.
    2. Make sure you fix typos like the repeated discription in class names which should be description. While a typo may seem like a small problem, if you work with other developers and they try to reuse your classname with the correctly spelled word, their css would not work because of the typo.
    3. Seems like there's an unnecessary amount of divs within the code. It would probable be useful to use other HTML5 tags instead of using div for every container. See: https://developer.mozilla.org/en-US/docs/Web/HTML/Element#content_sectioning
    4. Don't know what the .color class is supposed to do. Is it really just overwriting the color property repeatedly?
    5. There's some console.log() statements that print "gg" to the console. Make sure log statements like that don't get into final builds, since in a corporate environement, those might expose sensitive information that you don't want outside users knowing about.
    Marked as helpful
    0
  • Glenda Joyce Reed•1,260
    @Glenda9031Submitted about 2 years ago

    Ok. Either I'm getting better with coding or this one was easy! I welcome any and all feedback! Thank you!

    Huddle Landing Page

    2
    dgjenni2•320
    @dgjenni2Posted about 2 years ago

    It appears that the social media link images are broken on the web deployment. This is likely because the src fields do not start with "./" which allows the code to know to start looking from the current directory aka folder and then looking for an images folder with the specific social media svg file.

    Marked as helpful
    0
  • dgjenni2•320
    @dgjenni2Posted about 2 years ago

    1. The button appears to not have a shadow implemented on it that appears in the design. If you've never worked with component shadows before, I highly recommend locking into it because it can create some great effects.

    2. The social media icons do not appear to have perfect circles around them and look like squished ovals. I think you can fix this easily by adding "fa-fw" to the font awesome icon class, which will make each icon a fixed width (and height). From there, you can move your border to the anchor tag (a) parent of the icon and get a nice, perfect circle.

    (For reference, I did something similar for 2 here with the inverse-button tag and fa-fw icons: https://github.com/dgjenni2/frontent-mentor-huddle/blob/master/style.css#L86 https://github.com/dgjenni2/frontent-mentor-huddle/blob/master/index.html#L64 )

    Marked as helpful
    0
  • shikoto•1,010
    @shikot0Submitted about 2 years ago

    This is my first time using an API so I suppose the code looks a bit sketchy however it appears to work on both Chrome and Firefox, which is where most people had issues.... If anyone has any advice on how to work with APIs your feedback would be appreciated.

    Advice generator app using fetch

    1
    dgjenni2•320
    @dgjenni2Posted about 2 years ago

    Nice, simple solution. The fetch looks good, except I'd remove the console.log() statement before the final commit so that you don't log backend information to the console. I might also try to use a single .then() to handle the response rather than chaining two together, but nothing technically wrong with how you implemented it.

    On the css, I would recommend using classes and using tag selectors more than just ids. While ids do work on a smaller project like this, class and tag selectors can apply more broadly and be reusable between components. Also, really would try to avoid the hard coded pixel heights and widths in the css and use REM or EM units instead.

    Overall, nice work and keep at it!

    Marked as helpful
    1
  • dgjenni2•320
    @dgjenni2Posted about 2 years ago

    The sizing around the transition of the medium query appears broken. Right when it transitions from mobile to desktop, the container gets shrunken down to a skinny column of text. If you avoid setting the max-width to 30% and just use a fixed max-width and margin: 0 auto, then the container should continuously grow as a percentage of the screen until that fixed, maximum size (say 780px) you would want it on the desktop view, and the margin css should keep it centered.

    I would also recommend min-width rather than max-width for the media query to make the css developed in a mobile first patten, which should allow you to minimize the amount of css overall.

    Marked as helpful
    1
  • dgjenni2•320
    @dgjenni2Posted over 2 years ago

    Congratulations on getting the site semi-responsive with the media query. If you're just switching between a laptop screen and a cellphone screen, it should change layouts responsively.

    But, like the comment mentioned above, the next stop to make it more responsive to various cellphone screens, tablet screens and computer monitors would be to change the width from specific pixel widths to percentages. When using a percentage, the element will expand and contract to that percent of its parent's size, and the main body of the page will base its percentage on the size of the screen.

    It may be a bit hard to understand, but there is a free, helpful course that explains a lot about responsive design that you may want to check out. The content is spread over 21 days, but it's thorough with practice and is explained very well. Linked here: https://courses.kevinpowell.co/conquering-responsive-layouts

    Another small bit of feedback is you can have an easy update to your css to make the social icons and the button change when your cursor hovers over them. If you add :hover after a specific css rule, like button:hover or .fa-brands:hover, then the css in the rule will take effect when the cursor hovers over the element. You could change the color / background color or change the opacity of the element to let the user know you can click to interact with the elements.

    Hope this helps!

    Marked as helpful
    0
  • dgjenni2•320
    @dgjenni2Posted over 2 years ago

    From what I've seen, fetch and axios libraries are popular for javascript API calls. Fetch I've seen in more vanilla js and jquery projects, while I've seen axios used primarily in react projects.

    1
  • dgjenni2•320
    @dgjenni2Posted over 2 years ago

    Nice solution with the fetch request and the animation on the reload button. I'd try to avoid hard-coded the specific widths and heights for the various components in pixels as much as possible; the design gets very weird at some of the smaller screen resolutions. Percentages, rems and ems are much better units than pixels in general.

    Marked as helpful
    1