Dave

Latest comments

• 

  P

  Jorge Maza•2,000

  @jormazlopSubmitted 8 months ago

  

  Product list with cart using Angular

  #angular#bootstrap#sass/scss#typescript

  1

  70
  

  P

  Dave•5,295

  @dwhensonPosted 5 months ago

  Hi - I'm also working on learning Angular - what you've done looks amazing to me!

  One small suggestion would be to make the button element fit the full width and height of it's container as on the initial button when nothing is added to the cart I have to click right in the centre of the button to make things work.

  But overall looks great!

  Marked as helpful

  0
• 

  Fin Mardi•250

  @Yakobus-MardiSubmitted 8 months ago

  

  result summary component with grid and flexbox

  1

  20
  

  P

  Dave•5,295

  @dwhensonPosted 5 months ago

  Nice work on this one - looks great to me! I guess I wouldn't think of the items in the summary card as headers in this case, and would rather go for a ul with them as li elements inside them, but I'm really nitpicking here!

  0
• 

  CY Chan•290

  @CYCHAN00Submitted over 2 years ago

  

  Single price grid component

  #react#tailwind-css#next

  1

  30
  

  P

  Dave•5,295

  @dwhensonPosted over 2 years ago

  Hey @CYCHAN00 👋

  Lovely job here, and not much to comment on 🙌, but here's a couple of things to think about:

  • Button vs link? One thing that's worth considering is whether an element is actually a button or a link . The most important thing is what the element will do not what it looks like. This page has a great summary and lots of useful links on this: https://css-tricks.com/buttons-vs-links/ I imagine clicking on 'sign up' would take me to a new page, if you also think that then it should be a link.

  • I'd also get rid of all those br tags inside the square on the bottom right and just put everything inside a ul and make each item an li. I think this would be more semantic as they seem to be a list to me. You will need to remove the default list styling, but once that's done things will appear as they are now, but be better for people using assistive tech to access your site.

  Otherwise, keep up the good work and great job! 🥳

  Cheers Dave

  0
• 

  Ahmed Arafa•120

  @nerdy-guySubmitted over 2 years ago

  

  Four card feature section (HTML + CSS)

  1

  10
  

  P

  Dave•5,295

  @dwhensonPosted over 2 years ago

  Hello Ahmend, 👋

  Nice work here! This is a tricky one, you've got some of the hard details like the top border sorted - that really gave me problems for a long while! Here's a few things you might like to consider:

  • You are using grid on the main section, which is great, but not really getting the maximum benefit from this property. If you're new to it I'd suggest checking it out as it can be super helpful.

  In this case, you are using grid, but then also using margin to position elements, I'd go for one or the other (in this case grid most likely!). I'd suggest trying a grid with 4 rows, and 3 columns. And then have each card span the two rows it needs to (so the top card would be rows 1 and 2, and the middle cards 2 and 3, and the bottom card rows 3 and 4 - you'll also need to specify which column they should go in).

  Then if you use the gap property, rather than margin you should have a nicely laid out desktop page. You can then change the grid layout with a media query for mobiles (I'd actually probably start with the mobile view and then put the settings above in a media query for desktop, but it doesn't matter really).

  This is a bit of a tricky one to explain, but I hope you get the gist of what I am trying to say. If not just let me know and I'll try and clarify!

  Cheers Dave

  Marked as helpful

  1
• 

  Someshwari rudra•150

  @someshwari-rudraSubmitted over 2 years ago

  

  News homepage

  #bootstrap

  1

  40
  

  P

  Dave•5,295

  @dwhensonPosted over 2 years ago

  Hey @Someshwari - Lovely job here.

  The site looks great and responds well. Here's a couple of points to think about:

  1. You are getting some error messages from the linter about missing heading levels. This is because you should use the <h1> headings as the single, main heading of your web page, followed by the <h2> headings, then the less important <h3> headings, and so on, without skipping a level.

  We shouldn’t use headings to make text look BIG or bold. Use them only to set out your document's heading and show the document structure, and then change things up with CSS after that.

  I approach this by first laying out the page using only HTML and only thinking about the document structure, not design at all, and then once done, I return to the page and use CSS to make things look how they should.

  This is important as many people using assistive tech to access your pages will navigate the site based on the heading structure. At the moment this wouldn’t work with your HTML.

  2. I wonder whether your button should actually be a link. I imagine that it will take me to another page to read more? If so it should be a link. Again, the most important thing is what the element will do not what it looks like. This page has a great summary and lots of useful links on this: https://css-tricks.com/buttons-vs-links/

  Hope this helps a bit and gives you something to think about for the next challenge!

  Keep up the good work. Dave

  1
• 

  P

  Christopher Adolphe•620

  @christopher-adolpheSubmitted over 2 years ago

  

  Coffeeroasters subscription site | React, Unit Tests, End-to-End Tests

  #cypress#react#react-testing-library#sass/scss#typescript

  2

  177
  

  P

  Dave•5,295

  @dwhensonPosted over 2 years ago

  Hi Christopher,

  Lovely, lovely job here! I'm also pretty new to React and have been learning it for a few months so it's nice to find a fellow student.

  I noticed a couple of problems with the final modal 1) Keyboard focus doesn't move there and 2) The body remains scrollable.

  I've been lucky to be an early test on Josh Comeau's Joy of React course and just yesterday did the modal lesson. It can be really hectic, but there's a couple of packages that can basically solve this for you:

  1. react-focus-lock - moves and locks focus to the modal
  2. react-remove-scroll - removes scroll from all other elememts

  I've not tried them on a project but they seem pretty simple to implement and it might be worth giving them a try?

  Cheers Dave

  Marked as helpful

  1