Spacing the columns to align both horizontally was a challenge, but googling and reviewing others similar designs helped.
raionus
@sinjin25All comments
- @Lefty3Submitted about 1 year ago@sinjin25Posted about 1 year ago
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 :(
- Check the style-guide.md. It'll say the font family and the weights required.
- Go to google fonts and search up the family (ex: https://fonts.google.com/specimen/Lexend+Deca?query=lexend+deca)
- Select the font weights required (400, 700)
- Follow the instructions on the right so:
<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 :)
0 - @Rares-29Submitted about 2 years ago
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.
@sinjin25Posted about 2 years agoThe 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 helpful0 - @Unal-InanliSubmitted about 2 years ago
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?
@sinjin25Posted about 2 years agoI 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
andexpense-component__balance-amount
expense-component__balance__
and then you'd have the submodulestitle
andamount
. 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 helpful1 - @LekeadeloyeSubmitted about 2 years ago
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.
@sinjin25Posted about 2 years agoQ. 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 (probablybody
). I attached a screenshot of where it is on googlehttps://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 helpful0 - @JacobMarshall0Submitted about 2 years ago
Is there any way I can improve my JavaScript?
@sinjin25Posted about 2 years agoI 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 useddata-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 helpful2 - @txhawgSubmitted about 2 years ago
OK, 2nd try at this challenge. I think I have a better understanding of some of the concepts and what the challenges require.
@sinjin25Posted about 2 years agoGood 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.
0 - @ahmedmiliSubmitted about 2 years ago@sinjin25Posted about 2 years ago
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 helpful0 - @LooceeSubmitted about 2 years ago
I hope I got the box-shadow property correctly. I also hope I was able to replicate the design correctly.
@sinjin25Posted about 2 years agoGood 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 helpful0 - @sinjin25Submitted about 2 years ago
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.
@sinjin25Posted about 2 years agoOh man I didn't actually finish. How do I delete this :(
EDIT: decent attempt after finishing the desktop version.
- double border-radius on card elements
- svg size is off
- "my balance" font-size slightly too large (use padding/margin instead)
- bottom-right text slightly too big; use padding to reduce the height of the div instead
- bar labels slightly too small
- add a tiny bit more padding on the bars so they're slimmer
0 - @ejionautSubmitted about 2 years ago
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!
@sinjin25Posted about 2 years agoGood 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 helpful0 - @BijitPikaSubmitted about 2 years ago@sinjin25Posted about 2 years ago
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 designYou 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.
0 - @CreixzSubmitted about 2 years ago
Hello guys, I would like to know some specific points and I hope you can help me.
- Did I correctly implement the BEM methodology?
- Is there any way to improve or optimize my code? Thank you for your time and best wishes to you.
@sinjin25Posted about 2 years agoThe 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 helpful1