Article preview component

Community feedback

• 

  T

  Grace•31,270

  @grace-snowPosted almost 4 years ago

  Hi Alex,

  At first glance (viewing on mobile) this solution looks pretty good. Just needs a bit of space around the card so it's not hitting the top of my viewport.

  On closer inspection of the code, I see some problems in the html and ways to improve your javascript and css.

  • in html I'm not too worried about this component starting with a h4, because it wouldn't normally stand alone on a page. But then it's followed by a h6 then a h3. That's a problem. Headings have meaning and need to go in order. If you want to control size and other styling, use classes, don't Change the semantics
  • also in html, your click target needs to be an interactive element, like a button. This makes it accessible.
  • and those social icons should be links, with labels
  • in js I find it hard to understand what exactly is going on, but can see you're targeting multiple elements to show/hide. A much simpler way would be on click to add one class to a parent element - something like is-open - and let that control the visibility and position of the share panel (via css).
  • in css, I recommend you look up what position absolute does in terms of DOM. Instead look up what the different position properties do and how layout methods like flexbox or grid can solve your page layouts. You shouldn't need or want to position absolute very much. I would only absolutely position the share block when visible in this challenge.
  • also css, try to only use class selectors if possible. Place classes on. Child elements if possible and avoid repetition if you can (eg what class could go on the social links to consolidate their shared css properties)

  Hope those are helpful tips. Looks like you're on the right path, keep going, keep learning

  0

  ---

  

  Alexey•390

  @alexeykuz-sysPosted almost 4 years ago

  Hi Grace, thanks for such a detailed comments and tips. Something to work on and keep going. I am not fully aware of is-open so if u have an example of the code, would be great. thanks

  0

  

  T

  Grace•31,270

  @grace-snowPosted almost 4 years ago

  @alexeykuz-sys I mean on click of the button that triggers the share block to show up

  • add one class to the whole card via javascript
  • in the css make that share block hidden by default (either display none, or visibility hidden and opacity 0)
  • when the class is present, use css in in your css selectors to show that share block

  So css could look something like:

  .share-container {
    display: none;
    // inc mobile position properties here... 
  } 
  
  @media (min-width: XXXpx) {
   .share-container {
      // desktop position properties inside media query
    } 
  } 
  
  .is-open .share-container {
    display: block;
  } 
  

  (class names can be whatever, I've just tried to give an example)

  All javascript would have to do is

  • grab button
  • grab card
  • function toggles a class on the card
  • listen for a click event on that share button and call the function

  Hope that makes sense.

  0