@Lefty3
Submitted
Spacing the columns to align both horizontally was a challenge, but googling and reviewing others similar designs helped.
@sinjin25
@Lefty3
Submitted
Spacing the columns to align both horizontally was a challenge, but googling and reviewing others similar designs helped.
@sinjin25
Posted
Hey Lefty, nice job on the project.
One thing that'll help you out a lot going forward is making sure you've got your fonts right. This is a big deal because different fonts have different heights which makes it difficult to get your design to match up with the design docs. The process is not very obvious when the fonts don't come on your system either so that tricks up a lot of people :(
<link rel="preconnect" href="https://fonts.googleapis.com">
<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
<link href="https://fonts.googleapis.com/css2?family=Lexend+Deca:wght@400;700&display=swap" rel="stylesheet">
is placed inside the <head>
and
font-family: 'Lexend Deca', sans-serif;
can be added as a CSS rule to .body {}
(or anywhere you want).
You aren't told very well if you're using the font correctly. On firefox you can go inspect a text node -> check for "fonts" (near computed) -> make sure the font family is correct and not falling back to times new roman or something. On chrome you can inspect a node. Checkout the "computed" tab and scroll down to the bottom where it says "rendered fonts". Make sure it's the correct one and not a fallback.
Good job and good luck on your next projects :)
@Rares-29
Submitted
I don't know if I did the javascript part right, it is my first project of javascript. More exactly this part of code: submit.addEventListener("click", function(){ document.querySelector(".selector").innerHTML = "You selected " + stars + " out of 5"; document.querySelector(".div-boss.first-div").classList.add("hidden"); document.querySelector(".div-boss.thank-you").classList.remove("hidden"); }), where I should display the thank you page.
@sinjin25
Posted
The js seems fine to me at least.
I think that your script itself could be improved. I would recommend putting it in the head then using the defer
attribute. This is non-blocking (according to https://www.w3schools.com/tags/att_script_defer.asp) and waits until the dom content is loaded to execute, so no need to put it in the body anymore.
Your loop readability might be improved by using a .forEach
but that is up to personal style.
Good work on the project :)
Marked as helpful
@Unal-Inanli
Submitted
I'm trying to solidify my knowledge and application of B.E.M class names and I was just wondering if you also find it difficult to just keep on finding the most descriptive name for that element. Is there a technique that will push me in the right direction?
@sinjin25
Posted
I also find BEM naming to be kind of tedious (still useful though). If you're running out of brain power for names maybe you could try double modules?
ex: instead of expense-component__balance-title
and expense-component__balance-amount
expense-component__balance__
and then you'd have the submodules title
and amount
. Maybe then you could be undescriptive while still being confident you avoid a naming collision?
I noticed you're not using sass. If you're using BEM it can make it far less horrible through nesting. Instead of writing out the full module for everything:
.expense-component__ {
&balance-amount {}
&balance-title{}
&stats-container {}
&stats-end {}
}
etc.
Marked as helpful
@Lekeadeloye
Submitted
Should I have used Id's instead of classes??
Should I have used a section tag in my html file instead of main??
How do you link fonts directly into the Css file using import??
Any other general tips would be much appreciated.
@sinjin25
Posted
Q. Should I have used Id's instead of classes??
No I think classes are best in this case. Ids should only be used once per page, so they reduce the reusability of anything you make.
I mostly just use them to target things easily in javascript. They can be used for styling but I think using BEM naming is more flexible.
Q. Should I have used a section tag in my html file instead of main??
I think main is fine. <section>
is part of semantic html. Sections should usually have a h1/h2/h3 inside of it or I believe it's considered an accessibility error. Since you don't have headers, it would be inappropriate I believe.
Q. How do you link fonts directly into the Css file using import??
If you're using google fonts (which is best for these challenges), you can copy and paste their code directly into your css file. The @import should be by itself. The font-family
should be inside a rule (probably body
). I attached a screenshot of where it is on google
https://gyazo.com/e2334adb17e22469a81d058149245a06
Q. Any other general tips would be much appreciated.
I like using this rule for this website:
position: fixed;
left: 50%;
top: 50%;
transform: translate(-50%, -50%);
I add this right before I submit. It should center whatever you put it on (the container, card, etc.). This helps line up your work for the design comparison.
I would 100% avoid putting styling directly on elements (ex: your main
main img
etc.). There aren't any issues here, directly styled tags are a nightmare. Any styling you do in the future has to specifically override the default tag styling you've added.
I'd also recommend using a reset.css file (you can find some on google). This helps reduce the variability between how your work looks on different browsers by removing most/all the styling from all tags.
Marked as helpful
@JacobMarshall0
Submitted
Is there any way I can improve my JavaScript?
@sinjin25
Posted
I think that this piece of code is pretty fragile: rating=ratingButtons[i].innerHTML
because if someone changed the html structure it would fall apart.
You could try textContent
or even more reliable would be to use an arbitrary attribute (I used data-value="3"
) to store it, that way the value is independent of the labeling.
For your event handling. I think you're using the old style of event listeners (not sure). I think the most commonly accepted way right now is using event listeners.
So someEle.addEventListener('click', () => { /* do something */ })
Marked as helpful
@txhawg
Submitted
OK, 2nd try at this challenge. I think I have a better understanding of some of the concepts and what the challenges require.
@sinjin25
Posted
Good job, you're really close to perfect.
It looks like the design has slightly more padding and the title text is blue.
Your font-size is a little too big.
One trick I like to use is counting how many words are on the last line of the text. For instance, in the design the last line is "next level" while yours is "the next level" which tells you that your font-size isn't quite right. This doesn't always work but is easier to do than eyeballing it.
@ahmedmili
Submitted
@sinjin25
Posted
Good attempt :)
If you look at the design doc for mobile, you can see that the image is actually cropped on mobile sizes. You can do this for mobile by setting a height on the img and using object-fit:
object-fit: cover;
max-height: 200px;
As for the colors and fonts, there is a 'style-guide.md' provided in the files you downloaded if you want to get the fonts and colors as they were in the design doc. You're also free to explore alternative styles as well
Marked as helpful
@Loocee
Submitted
I hope I got the box-shadow property correctly. I also hope I was able to replicate the design correctly.
@sinjin25
Posted
Good job
I think your image is broke on the github deployment.
I was able to fix it on github by changing the url: url(..icon-view.svg) to url(icon-view.svg)
A bit of padding to the right of the avatar and a bit less margin on top of the decorative bar might get it a little closer
Marked as helpful
@sinjin25
Submitted
Vue solution abusing slots. Used grid instead of flex on the chart itself because I designed it last (so I had I knew the height it had to be ahead of time).
Questions: Any feedback on how to modify the size of the svg while keeping the aspect ratio? I literally just modified the height and width which didn't exactly give the results I wanted.
@sinjin25
Posted
Oh man I didn't actually finish. How do I delete this :(
EDIT: decent attempt after finishing the desktop version.
@ejionaut
Submitted
I'm still currently playing around with grids. For this exercise, I am not sure why the container wasn't wrapping when I adjusted the size. So I just set the column flow to row when adjusted.
Any feedback is welcome!
@sinjin25
Posted
Good job, especially on the the padding between card sections. I messed that up on my solution.
I think that your font-size is slightly too big on the body text and the button text is slightly too small. Very good work though.
You might want to use a position: fixed on the attribution text so that it's taken out of the document flow. This will help your solution line up with the design on the design comparison
Marked as helpful
@BijitPika
Submitted
@sinjin25
Posted
Good job on the text!
You can use the letter-spacing
attribute on the "perfume" text to make the letters appear more far apart like in the design
You can use flexbox with align-items: center
on the add cart button to vertically center the cart and text.
The primary price tag (not the crossed-out one) should be the same font and the "Gabrielle..." title text.
@Creixz
Submitted
Hello guys, I would like to know some specific points and I hope you can help me.
@sinjin25
Posted
The BEM seems correct.
I'd recommend putting padding on the card rather than on each element inside the card. This will let you standardize the padding instead of having to set in everywhere. The method you did makes it hard to make changes.
You can get away with just having this on the image:
max-width: 100%
border-radius: (whatever)
That lets you get away from using a magic number for width.
grid-template-rows with percents seems like a bit of a fragile solution. I'd recommend splitting it like: card -> card-head + card-body and then maybe using fr to divide up the space between the head and body.
Marked as helpful