@nkirukka
Submitted
Any idea how I can remove the grid gaps in the gallery section on mobile screen? I tried using the gap property but it didn't work.
@DreamCoder7
@nkirukka
Submitted
Any idea how I can remove the grid gaps in the gallery section on mobile screen? I tried using the gap property but it didn't work.
@DreamCoder7
Posted
Hey, Nkiruka Ebele👋
Great effort on this challenge!👏
A few things I'd like to suggest are,
Hope you find this helpful!😊
Happy coding😎
@BreinerJT
Submitted
This challenge took me a while because I don't know much about async / await functions, I don't feel good about my knowledge in this kind of functions so if you know where can I learn or read more about this I would really appreciate it if you share it with me. Any feedback is welcome!
@DreamCoder7
Posted
Hey, BreinerJT👋
Nice effort on this challenge!👏
Overall, your solution looks great! One small thing I'd like suggest is that instead of outputting the chart data one by one using html like you did, you can dynamically output from JavaScript. Also you can add a Gaurd clause incase if the user failed to load, More about Gaurd clause
async function getData(url){
let fetchData = fetch(url)
let response = await fetchData;
// Adding Gaurd clause
if (!response.ok) return;
let data = await response.json()
return data;
}
async function getCharts(){
const chart = document.querySelector(".charts");
const data = await getData('data.json');
let html = null;
data.forEach(e => {
html = `
<div class="chart-container">
<p class="day" data-id="mon">${e.day}</p>
<div class="js-chart chart" id="mon" style="height: ${e.amount}%"></div>
<label class="js-label" for="mon">${e.amount}</label>
</div>
`
chart.insertAdjacentHTML("afterbegin", html);
});
// Not needed
/*
const amounts = data.map(e => e.amount)
let maxValue = Math.max(...amounts)
let heights = amounts.map(e => e / maxValue * 100)
charts.forEach((value, index) => {
value.style.height = `${heights[index]}%`
})
charts[day].classList.add("current-day")
labels.forEach((value, index) => {
value.textContent = `$${amounts[index]}`
})
*/
}
getCharts()
Hope that helps!🤗
Happy coding.😎
Marked as helpful
@System-Fresh
Submitted
@DreamCoder7
Posted
Hi, system-fresh👋
Good effort on this challenge!👌
Here are a few things I'd like to suggest,
<section class="productbig">....</section>
because it's more sense to have it's own section. And also wrapping the testimonial part like this is also practical<figure class="box">
<div class="profile"></div>
<blockquote>
<p>...</p>
</blockquote>
</figure>
<img src=".." alt=".."/>
to display the icon-star.svg you can use the css background-image
to avoid the repetition you made on HTML.background-image: url('..');
background-size: 20px 20px;
background-repeat: repeat-x;
background-repeat: space;
....
padding: 0;
and 'margin: 0;' using the universal selector (*
) to reset all the element means any style that you put in the body gets inherited you don't need to repeat into the body{}
tagHope you find this helpful.😊
Keep coding!😎
@Tahmeed-programmer
Submitted
@DreamCoder7
Posted
Hello, Tahmeed!👋
Nice work on this challenge!👏
Here are a couple of things I'd like to suggest,
.top-left-head ul li
using class or id is more practical wayHope that help!😊
Happy coding!😎
@dannyswagg
Submitted
@DreamCoder7
Posted
Great job, Ugiomoh Daniel
But there is a little bit of a problem with your semantics usage. you use a div element whenever you want to use a wrapper or container. The problem with this is it's difficult for screen readers. You can fix or prevent this kind of problem using WAI-ARIA Roles. ARIA roles provide semantic meaning to content, allowing screen readers and other tools to present and support interaction with object in a way that is consistent with user expectations of that type of object. For more
And I also notice that you use two heading element twice and that isn't a good practice. Here are some guidance that should help:
/*
This is the two heading that use the same heading element and it's not mandatory but instead of using h1 in this case you can use h4 or h5 because h1 indicates the most important (or highest-level) heading on the page.
*/
/*
<h1>How did we do?</h1>
<h1 class="ht">Thank you!</h1>
*/
/* Instead of using onclick="" you can select the parent element and by using Event propagation you can do the same thing but applying DRY principle.*/
/*
<div class="numbers" id="numbers">
<button onclick="star1()" id="A" class="botn">1</button>
<button onclick="star2()" id="B" class="botn">2</button>
<button onclick="star3()" id="C" class="botn">3</button>
<button onclick="star4()" id="D" class="botn">4</button>
<button onclick="star5()" id="E" class="botn">5</button>
</div>
*/
const parentBtnEl = document.querySelector('.numbers');
parentBtnEl.addEventlistener('click', function(e) {
const btn = e.target;
if (btn.closest('A')) {
// Do something....
}
......
});
Dificuldade em deixar o código responsivo
@DreamCoder7
Posted
Great job, Letícia dos Santos,
But there is a big problem with semantics. I noticed that all the images don't have alt attribute that is one of the reason you have an ACCESSIBILITY issue and also you should only use id for unique purposes instead you can use class.
Here are some guidance that should help:
/*
id: is unique in the context of a web page; It can not be duplicated.
class: can appear multiple times on multiple element.
[For more explanation: ] (https://css-tricks.com/the-difference-between-id-and-class/)
[More: ] (https://www.w3schools.com/html/html_id.asp)
*/
/*
use alt attribute:
<img src="./images/icon-ethereum.svg" alt="Ethereum Icon"/>
*/
/*
// Basic setup using universal selector feel free to include at the top of your CSS code
*,
*::before,
*::after {
margin: 0;
padding: 0;
box-sizing: border-box;
}
The Universal Selector (*): Literally the asterisk character. It is essentially a type selector that matches any type. Type meaning an HTML tag like <div>, <body>, <button>, or literally any of the others.
*/
@Willians45
Submitted
@DreamCoder7
Posted
great job, Willians;
But I see some issue with your styling and structuring your HTML file. First separating your style from HTML or ignoring inline style is way better for reading your code. Second you have a lot of accessibility issue because of your HTML structure.
Here are some changes I made in browser that should help with accessibility issue:
/*
<a href="">
<img class="img" src="/images/image-equilibrium.jpg" alt="" />
</a>
*/
// The alt attribute is a semantic html (means a certain element have actually meaning and purpose attached to them)so don't ignore it. it's use full for accessibility and also SEO
<a href="#">
<img class="img" src="/images/image-equilibrium.jpg" alt="some image" />
</a>
<div class="lado">
// why bothering using (<h4></h4>) and also even if you want to use you should use (<h3></h3>) tag because Heading levels are important!
/*<h4><img class="img1" src="/images/icon-ethereum.svg" alt="">0.041ETH</h4>*/
// you can fix like this:
<img class="img1" src="/images/icon-ethereum.svg" alt="ethereum Icon">
<p>0.041ETH</p>
</div>
/* hr {
background-color: hsl(215, 32%, 27%);
border-color: hsl(215, 32%, 27%);
height: 1px;
width: 90%;
} */
// instead of using <hr /> tag you can do it like this. but still if you prefer using <hr /> tag you can use but it better to use like this
.lado2 {
border-top: 2px solid hsl(215, 32%, 27%);
}
/* Why??
img.img2 {
vertical-align: middle;
}*/
// Instead do this
.im2 {
vertical-align: middle;
}
Marked as helpful
@stephanniegb
Submitted
feedback needed
@DreamCoder7
Posted
Well done, great job.
But there is problem with your styling. First instead of using inline style you can use external file and link to your HTML that way it's better to view your code. Second I kind of looking your styling code and I found that some of your code not necessary or duplicated and also messing some style.
Here are some changes I made in browser that should help with remove unnecessary code:
/*You should name your HTML tag as necessary for the feature*/
div{
text-align: center;
}
/*This is the parent element*/
main {
display: flex;
justify-content: center;
}
.card {
background-color: white;
/* Not necessary you can center your card by styling the parent element */
/* display: flex;*/
/*justify-content: center; */
/* flex-direction: column; */
/* margin-left: 500px; */
/* margin-right: 500px; */
/* padding-top: 15px;*/
/*padding-left: 5px;*/
/*padding-right: 5px; */
padding: 15px 5px 0 5px; // You can do it in one line
border-radius: 10px;
box-shadow: 0 4px 8px 0 rgba(0, 0, 0, 0.2);
transition: 0.3s;
}
```;
Marked as helpful