All comments

• 

  Dan Bennett•60

  @inkfrombloodSubmitted over 2 years ago

  What I found difficult: Figuring out how to fit the QR code into the container .div effectively. Turned out to be as simple as setting the width to 100%.

  What I'm unsure of: That this approach was the most efficient way of achieving this. When to use px or when the rem.

  I edited the footer links to have a hover state of white, but wanted to change the visited color from purple to blue, however when I changed the visited state it removed the hover state. I'm unsure of how to make this happen.

  

  QR code component solution

  2

  40
  

  Mo•840

  @MohamedAridahPosted over 2 years ago

  Hello @inkfromblood, congratulation on submitting your first solution🎉👏.

  Yes giving the QR code image max-width: 100% is totally right since this will make the image to take the full width and never overflow it.

  I have also some notes for you:

  • use font-family: 'Outfit', sans-serif globally instead of redeclare every time:

  body {
      font-family: 'Outfit', sans-serif;
  }
  

  • instead of using <h2> heading level for the text you can just use <p> element since this text is not a heading. This will fix your Accessibility Issues

  • use <footer> instead of .attribution div it's more HTML Semantic. Also This will fix your Accessibility Issues

  • This challenge Doesn't has nay links so you can Remove styles of the <a> tag since it's useless.

  • you can see My solution for this challenge it may be helpful for you..!

  I hope this wasn't too long for you, hoping also it was useful😍.

  Goodbye and have a nice day.

  Keep coding🚀

  Marked as helpful

  0

  This comment has received 1 replyShow
• 

  Moruf Lawal•10

  @MorufLawalSubmitted over 2 years ago

  i find it difficult creating the hover effect on the image

  

  responsive landing page using CSS Flex

  2

  30
  

  Mo•840

  @MohamedAridahPosted over 2 years ago

  Hello @MorufLawal, congratulation on submitting your first solution🎉👏.

  I have some notes for you:

  • for the hover effect, put the image with .eqn class inside div with .image-wrapper class for example and follow these styles:

  .image-wrapper::after{
  	content: url(./images/icon-view.svg);
  	position: absolute;
  	background-color: rgb(0 255 247 / 45%);
  	width: 100%;
  	height: 100%;
  	opacity: 0;
  	top: 0;
  	left: 0;
  	display: flex;
  	align-items: center;
  	justify-content: center;
  	cursor: pointer;
  	border-radius: 1rem;
  	overflow: hidden;
  	transition: .3s ease;
  }
  

  .image-wrapper:hover::after {
      opacity : 1;
  }
  

  i used div to wrap the image and do the hover effect since <img> alone doesn't work with ::after pseudo element.

  • instead of using <hr> tag you can use border-top or border-bottom depending on the div you will use it. Also this is more Semantic

  • for .Bottom give it text-align: start to be like the design also give space between the avater image and the text.

  • use <a> element instead of span for author name. It's more Semantic and on click this name this may lead to another page.

  • you can see My solution for this challenge it may be helpful for you..!

  I hope this wasn't too long for you, hoping also it was useful😍.

  Goodbye and have a nice day.

  Keep coding🚀

  0
• 

  Sultan Hafizh Alexander•30

  @shalexandeerSubmitted over 2 years ago

  how to add button shadows in proceed payments?

  

  order-summary-component-main

  2

  40
  

  Mo•840

  @MohamedAridahPosted over 2 years ago

  Hello ,

  • for .parent you need to add background-color property to to add background for the element and make it like the design.

  • for .parent there is no need to use fixed height. height: 560px.

  • for .parent to make the waves illustration looks closer to the design you can change background-size property to 100% instead of cover. or you can remove background-size property at all. you cab read about background-size from Here.

  • So after all this changes to the .parent it's styles will be like this:

  .parent {
      display: grid;
      place-items: center;
      min-height: 100vh;
      background: #e0e8ff url(../images/pattern-background-desktop.svg) no-repeat;
  }
  

  • for .container you can remove margin-top property if you want the card to be perfectly centered.

  • Don't use fixed height let content itself determine it's height. so you can remove height: 335px for .card-content div

  • for card-title instead of using <h2> heading level use <h1> heading level instead. This will fix your Accessibility issues as well.

  • when you use width: 80% instead of using margin-left or margin-right to center the content . You can just use margin: auto.

  • use font-family globally in the body for example instead of redeclare it for each element

  • .pay and .cancel have common styles so you can combine them in one class and add it to them instead of redeclare it again.

  • for your question you can see My solution for this challenge it may be helpful for you..!

  I hope this wasn't too long for you, hoping also it was useful😍.

  Goodbye and have a nice day.

  Keep coding🚀

  0

  This comment has received 1 replyShow
• 

  P

  Kent O'Sullivan•1,870

  @12KentosSubmitted over 2 years ago

  The way I have the code written, when the Eye is hovered, the teal background stops showing, I wasn't able to figure out a good way to fix this. Any tips would be appreciated.

  Thanks!

  

  More practice with Figma

  2

  50
  

  Mo•840

  @MohamedAridahPosted over 2 years ago

  Hello @12Kentos, Good job it's almost identical to the design👍👏

  However i have some notes for you:

  • use min-height: 100vh for the body instead of using height: 100vh. This will allow your content to take more vertical height if content needed to. So user can see all the content.

  • instead of using these styles for .eth-cube-img

  .eth-cube-img {
      width: 30.02rem;
      height: 30.02rem;
      border-radius: 0.8rem;
  }
  

  you can just give the <img> the full width and the padding of .eth-card-container will make sure that there are space around the image. So your styles for .eth-cube-img could be:

  .eth-cube-img {
      width: 100%;
      border-radius: 0.8rem;
  }
  

  • for the hover effect you can Delete .eth-eye-img img tag and .eth-img-container styles and try pseudo elements like ::after or ::before like:

  .eth-img-container::after{
  	content: url(./images/icon-view.svg);
  	position: absolute;
  	background-color: rgb(0 255 247 / 45%);
  	width: 100%;
  	height: 100%;
  	opacity: 0;
  	top: 0;
  	left: 0;
  	display: flex;
  	align-items: center;
  	justify-content: center;
  	cursor: pointer;
  	border-radius: 1rem;
  	overflow: hidden;
  	transition: .3s ease;
  }
  

  .eth-img-container:hover::after {
      opacity : 1;
  }
  

  • instead of using <p> element for .eth-title you can use <h1> because this is a a heading.

  • use <a> element instead of span for .eth-auth-name. It's more Semantic and on click this name this may lead to another page.

  • you can see My solution for this challenge it may be helpful for you..!

  I hope this wasn't too long for you, hoping also it was useful😍.

  Goodbye and have a nice day.

  Keep coding🚀

  Marked as helpful

  1
• 

  tbrownlee•350

  @tbrownleeSubmitted over 2 years ago

  Was using em in the media query the best solution to change the font-size?

  

  Basic HTML and CSS (with Flexbox)

  1

  30
  

  Mo•840

  @MohamedAridahPosted over 2 years ago

  Hello @tbrownlee, you did Great for this challenge, it's also Responsive so good job.👏👍

  • your styles to .main is extra and has no purpose. So Remove it.

  • using flex with the <img> is also can be Removed. the <img> element isn't parent for an any element so display: flex will do nothing.

  • instead of giving width property to the .text or the <img> you can just let them take 100% of the width and use padding property to control and give the outer space around the .container.

  • it's better to use rem units for font-size and somethings like that. em is relative to the font-size of its direct or nearest parent . rem is relative to the HTML (root) font-size. Read this Article for better understanding.

  • you can see My solution for this challenge it may be helpful for you..!

  I hope this wasn't too long for you, hoping also it was useful😍.

  Goodbye and have a nice day.

  Keep coding🚀

  Marked as helpful

  1

  This comment has received 1 replyShow
• 

  tbrownlee•350

  @tbrownleeSubmitted over 2 years ago

  Is there a better way to size the icons rather than manually selecting a height and width in pixels?

  Is there a better way to shrink the elements for the desktop view rather than manually changing the font/ height and width of each one?

  

  Mobile-first solution using Flexbox

  2

  30
  

  Mo•840

  @MohamedAridahPosted over 2 years ago

  @tbrownlee I'm glad this was useful for you🌹👍. you can read more about pseudo elements from Here.

  • Yes, you can change the icon size by using background-size property.

  .interactive::after {
      background-size: 55px; /* you can use percentage % as well */
  }
  

  You can read more about it from Here.

  Keep Coding🚀

  Marked as helpful

  1

  This comment has received 1 replyShow
• 

  Kratos012•90

  @Kratos012Submitted over 2 years ago

  Feedback plz!!! NB: My css is messy. Was tired working on this. Just wanted to hit my daily goals.

  

  single-price-grid-component Using Grid

  2

  20
  

  Mo•840

  @MohamedAridahPosted over 2 years ago

  Hello @Kratos012, you did very Good job for this challenge it's also Responsive so Well done👏👍.

  however i have some notes for you:

  • Please Recheck your solution background colors and make it like the design.

  • To insure .heade will take the full width you can use:

  .header{
  	grid-column: 1/-1;
  }
  

  your current way is totally right

  • you should use <h1> heading level once in the page. So instead of using <h1> tag inside .subscribtion div you can use <h3>. because the main heading of this div is <h2> and you should use heading levels by orders like: using <h1> then <h2> then <h3> and so on. This is Useful for SEO

  • For p element inside .whyus div. Try use ul element (unordered list) will be much better and this is More Html Semantic

  	<ul>
  		<li>Tutorials by industry experts  </li>
  		<li>Peer & expert code review </li>
  		<li>Coding exercises</li>
  		<li>Access to our GitHub repos</li>
  		<li>Community forum </li>
  		<li>Flashcard decks </li>
  		<li>ew videos every week</li>
  	</ul>
  

  • Your CSS code is Good ,but your styles for <h1>, <h2>, <p> and <a> are set Globally . I've just wanted to draw your attention to this

  I hope this wasn't too long for you, hoping also it was useful😍.

  Goodbye and have a nice day.

  Keep coding🚀

  Marked as helpful

  0

  This comment has received 3 repliesShow
• 

  tbrownlee•350

  @tbrownleeSubmitted over 2 years ago

  Is there a better way to size the icons rather than manually selecting a height and width in pixels?

  Is there a better way to shrink the elements for the desktop view rather than manually changing the font/ height and width of each one?

  

  Mobile-first solution using Flexbox

  2

  30
  

  Mo•840

  @MohamedAridahPosted over 2 years ago

  Hello @tbrownlee, you did Good job. and Your solution is Also Responsive.👍👏 it seems quite small to me but its ok.

  I have some notes for you:

  • inside .interactive div instead of using .interactive-overlay and .interactive-icon. You can use pseudo element instead Your way is totally right but you can try the following:

  .interactive::after{
  	content: url(./images/icon-view.svg);
  	position: absolute;
  	background-color: rgb(0 255 247 / 45%);
  	width: 100%;
  	height: 100%;
  	opacity: 0;
  	top: 0;
  	left: 0;
  	display: flex;
  	align-items: center;
  	justify-content: center;
  	cursor: pointer;
  	border-radius: 1rem;
  	overflow: hidden;
  	transition: .3s ease;
  }
  

  .interactive:hover::after{
  	opacity: 1;
  }
  

  • instead of using <hr> tag you can use border-top or border-bottom depending on the div you will use it. Also this is more Semantic

  • use <a> element instead of span for author name. It's more Semantic and on click this name this may lead to another page.

  • you can see My solution for this challenge it may be helpful for you..!

  I hope this wasn't too long for you, hoping also it was useful😍.

  Goodbye and have a nice day.

  Keep coding🚀

  Marked as helpful

  1

  This comment has received 1 replyShow
• 

  gurpreet•280

  @justcoder42022Submitted over 2 years ago

  good and first time on project CSS positioning experience

  

  NFT preview card component by Gurpreet Singh Kalyan

  1

  50
  

  Mo•840

  @MohamedAridahPosted over 2 years ago

  Hello @justcoder42022, impressive.! your solution layout is very good and almost similar to the design.

  However i have some notes for you:

  • instead of using display property to control showing and hiding of the icon on the image on hover. You can use for example opacity. This will allow you to use transition property with it and hover will be much smoother. Since display doesn't work with transition.

  .main-image-section::after {
          left : 0; /* to indicate starting point horizontally   */
          opacity: 0; /* use opacity instead of display */
          transition : opacity 200ms linear;
      }
  

  .main-image-section:hover::after {
          opacity: 1; 
          cursor : pointer; /* better than the default and design did that too */
      }
  

  • For .main-image-section::after you can use background property shorthand like:

  .main-image-section::after {
          background: hsla(178, 100%, 50%, .6) url("images/icon-view.svg") no-repeat center;
      }
  

  • For .main-image-section::after instead of using height: calc(100% - 4px). You can just give the <img> display : block property and everything will work fine.

  .main-image-section::after {
         height: 100%;
      }
  

  .main-img {
         display: block;
      }
  

  • instead of using strong element inside .avtar-containter div. You can use <a> element. It's more **Semantic ** and on click this name this may lead to another page. Also don't forget to add transition for it on hover.

  • using:

  li {
    list-style: none;
  }
  

  this way means you want to use it Globally. If so, it's better to put it at the beginning of the code. Optional and Your way is totally right

  • you can see My solution for this challenge it may be helpful for you..!

  I hope this wasn't too long for you, hoping also it was useful😍.

  Goodbye and have a nice day.

  Keep coding🚀

  Marked as helpful

  1

  This comment has received 1 replyShow
• 

  robinjmm•390

  @robinjmmSubmitted over 2 years ago

  Any feedback on how I can improve my code?

  

  NFT Card Component Solution

  2

  70
  

  Mo•840

  @MohamedAridahPosted over 2 years ago

  Hello @robinjmm,

  I have some notes for you:

  • use <a> element instead of span for .nft-card__creator-name. It's more Semantic and on click this name this may lead to another page.

  • you can just use <img> instead of .nft-card__eth::before and .nft-card__timer::before. the current way is totally right.

  • instead using both ::after and ::before pseudo elements you can use one of them as the styles below and everything will work fine:

  .nft-card__bg-layer::after {
          content: url(./images/icon-view.svg);
          position: absolute;
          background-color: rgb(0 255 247 / 45%);
          width: 100%;
          height: 100%;
          opacity: 0;
          top: 0;
          left: 0;
          display: flex;
          align-items: center;
          justify-content: center;
          cursor: pointer;
         border-radius: 1rem;
          overflow: hidden;
          transition: .3s ease;
  	}
  

  .nft-card__bg-layer:hover::after {
          opacity: 1;
  	}
  

  • use transition property for smoother hover effect for both .nft-card__heading and .nft-card__creator-name

  • you can see My solution for this challenge it may be helpful for you..!

  I hope this wasn't too long for you, hoping also it was useful😍.

  Goodbye and have a nice day.

  Keep coding🚀

  Marked as helpful

  1

  This comment has received 2 repliesShow
• 

  Milos Djordjevic•990

  @milosshomySubmitted over 2 years ago

  Any feedback?

  

  QR Code using html css

  1

  40
  

  Mo•840

  @MohamedAridahPosted over 2 years ago

  Hello @milosshomy,

  I have some notes for you:

  • don't use height for .card this causes big space in the bottom of your card. let content inside the card take the suitable spaces like padding and margin instead.

  • when you remove height for the .card div you will need to add padding-bottom for the .card-content div

  • remove margin-bottom for the image. you can use it for .card-image div itself.

  For Accessibility issues:

  • wrap .card div inside <main> tag.

  • use <h1> heading level instead of using <h3> heading level.

  • you can see My solution for this challenge it may be helpful for you..!

  I hope this wasn't too long for you, hoping also it was useful😍.

  Goodbye and have a nice day.

  Keep coding🚀

  Marked as helpful

  0
• 

  Khalil Nazari•220

  @khalilnazariSubmitted over 2 years ago

  Dear members,

  another challenge is resolved. Please share your idea if any improvement is needed.

  Cheers!

  

  QR code card component

  1

  60
  

  Mo•840

  @MohamedAridahPosted over 2 years ago

  Hello @khalilnazari, your solution is very impressive, and your code is clean enough.

  • the only note for you is to use min-height: 100vh for the body instead of using height: 100vh. This will allow your content to take more vertical height if content needed to. So user can see all the content.
  • you can set text-align directly to .card .card__body div instead of h1 and p. because they both share the same value. The current solution is totally fine and right

  For Accessibility issues

  you can just put .container div inside <main> tag. This should fix your accessibility issues.

  I hope this wasn't too long for you, hoping also it was useful😍.

  Goodbye and have a nice day.

  Keep coding🚀

  Marked as helpful

  0

  This comment has received 2 repliesShow