NFT preview card component

Community feedback

• 

  Elir-Mahad•100

  @Elir-MahadPosted about 3 years ago

  Hey there :-) !

  You did a great job. Your code is well organized, clean, and good. I had no trouble understanding what you did and this is always a good thing.

  I looked through your css and i noticed that some improvements could be made.

  General feedback:

  • Try to write your css in the same format of your html.
  • Example: Since your attribution is at the bottom of the html, then the attribution styles should be at the bottom of css.

  Specific feedback:

  All of my feed back is in the code below. Here is what the abbreviations mean:

  R: Stands for 'Remove'.

  • You should remove this line because when i put it in a comment, it had no effect on your component.

  MC: Stands for 'my comment'. Here i am talking to you directly.

  body {
  	background-color: var(--very-dark-blue-main-bg);
  /* R:	position: relative; */
  	font-family: "Outfit", sans-serif;
  	font-family: "Signika Negative", sans-serif;
  }
  
  /* .......Card.......... */
  
  .card {
  /* R:	position: absolute; */
  /* R:	top: 30px; */
  /* R:	right: 0; */
  /* R:	left: 0; */
  /* R:	display: flex; */
  /* R:	align-items: center; */
  	background-color: var(--very-dark-blue-card-bg);
  /* R:	justify-content: center; */
  /* R:	flex-direction: column; */
  /* R:	border-radius: 10px; */
  	padding: 2rem;
  	margin: 0 auto;
  	width: 330px;
  	height: 555px;
  	
  	/* MC: I added the margin-top below 	 */
  	margin-top:3rem;
  	
  }
  
  /* ...... Main image........ */
  
  .card-top img {
  	object-fit: cover;
  	width: 100%;
  	height: auto;
  	border-radius: 5px;
  }
  
  /* .......First content .......... */
  
  .card-content > h2 {
  	color: white;
  	font-size: 25px;
  	line-height: 2;
  }
  
  .card-content > p {
  	color: var(--soft-blue);
  	font-size: 17px;
  }
  /* ...........Price................... */
  
  .card-price {
  	display: flex;
  	justify-content: space-between;
  	align-items: center;
  /* R: text-align: center; */
  	margin: 1rem 0;
  }
  
  /* MC: Below i combined price-left and price-right because they have the same styles. 
     The text and image were not aligned in your version, so i aligned them using flex.
  */
  
  .price-left, 
  .price-right{
  	color: var(--yan);
  	display:flex;
  	justify-content: space-between;
  	align-items:center;
  }
  
  /* MC: You should give the icon in the price-left a class 
  and then give it margin-left:0.5rem; */
  
  /* card-owner */
  
  /* MC: I placed the hr here to match the flow of the design. */
  
  hr {
  	height: 1px;
  	border: none;
  	background-color: var(--very-dark-blue-line);
  }
  
  .card-owner {
  	display: flex;
  	align-items: center;
  	margin-top: 1rem;
  }
  .card-owner img {
  	width: 20%;
  	border-radius: 50%;
  	border: 1px solid wheat;
  	margin-right: 1rem;
  }
  
  .card-owner p {
  	color: var(--soft-blue);
  	font-size: 15px;
  }
  
  .card-owner p span {
  	color: white;
  }
  
  /* Attribution below */
  
  /* MC: I placed the attribution here to match the flow of the design. */
  
  .attribution {
  	color: var(--yan);
  	font-size: 15px;
  	text-align: center;
  	position: fixed;
  	bottom: 0;
  	right: 0;
  	left: 0;
  }
  .attribution a {
  	color: var(--yan);
  }
  

  Keep up the good work !!

  Marked as helpful

  1

  ---

  

  NKUSI KEVIN•30

  @nkusikevinPosted about 3 years ago

  @Elir-Mahad Thank you very much . I really I appreciate the reviews you gave me . Am going to try and make all the improvements .

  0

  

  Elir-Mahad•100

  @Elir-MahadPosted almost 3 years ago

  @nkusikevin Your'e welcome my friend. Keep up the good work !!!! :-)

  Marked as helpful

  0