Yunus emre Salcan• 160
@Yemresalcan
Submitted
Cesar D.
@ThatDevDiaz
All comments
- Cesar D.• 400
@ThatDevDiaz
Posted
Hey! great solution.
For the validation checks, you can clean up a lot of the code by creating an error function. Ex: function errorCheck(){ }; This errorCheck function can then be called anytime the input value does not meet the criteria instead of writing individual errors for every validation check. This will clean up the code a bunch. You can place d, m, y in an array and callback this error depending on whichever validation check they failed. Just an idea for a different approach.
Your CSS and HTML was definitely a lot closer than mine that's for sure!
Great stuff. Was cool seeing a different approach to this. I learned some from your code. Keep on coding
Marked as helpful
1 - Leonardo Henrique• 130
@henrikkudesu
Submitted
I couldn't (or didn't try?) to animate the numbers. I still don't know how to do it, if I can do it in pure CSS or with Javascript.
Cesar D.• 400@ThatDevDiaz
Posted
I'm not 100% sure what you mean by animating the numbers but,
if you're referring to the " - -" and replacing it with the results, you should create an element : <span id="yearsResult">- -</span>
then in Javascript you're going to target this ID and store it like this:
let years = document.getelementbyid(
yearsResult);Once stored, you're going to retrieve the results from (what I assume should have been the function calculating the years) and replace the span value with the results using basic dom manipulation. Store the results in a variable so you can then use template literals to display the results. Lets say you stored years in a variable called "calculatedYears". You can then do :
years.innerHTML =
${calculatedYears};and your span value will now display whatever the calculatedYears value returned from the calculation function.
Hope this helped.
Marked as helpful
0 - vintech05• 370
@vintech05
Submitted
any feedback would be greatly appreciated regarding the most efficient use cases with javascript for this particular project. I definitely did violate the DRY(don't repeat yourself) rules here.
Cesar D.• 400@ThatDevDiaz
Posted
Hey!
I noticed you labeled each individual number and applied a function to each different selection
One thing I would do to help with the dry principle is to just create a single function which will receive the user's input as a value and return this value so you can then call this function in a template literal and the result will be dynamic instead of statically creating the individual outcome for each different click. In this case, the user's input will only be recorded once the submit button is pressed. No need to record whats pressed every time because it really only matters once they hit submit.
The end result will be something like - ratingResult.textContent = "You've selected ${selectedButton()} out of 5!"
and of course, selectedButton will be whatever value the function returned. This way you only have to make the event listener for the "submit" button and not an event listener for every individual rating.
Marked as helpful
1 - Sumith Wickramasinghe• 40
@sumithkck
Submitted
Cesar D.• 400@ThatDevDiaz
Posted
Hey!
Try using height: 100VH in your header to set the viewport high to 100%. Then you can use flexbox on the body to center the middle using justify-content: center; and align-items: center; . This will revolve your issue with the qr code not being centered.
Also, try using font-weight to change the bold on the text so it better matches the original design. Pretty close though
Marked as helpful
0 - Lehelac• 40
@Lehelac
Submitted
- How can I make sure that the DIV is centered regardless of the computer screen size?
- Is there a way to make the website responsive with only one line of code instead of individually specifying each screen size within the code?
Cesar D.• 400@ThatDevDiaz
Posted
Hey!
When working with projects, always make sure in these kind of situations to utilize 100% of the viewport. The viewport is the entirety of the screen/page.
You wanna set the viewport to 100% by using :
body { height: 100vh; }
There's other ways to do this as well, this is just one way. Once the viewport height is set to 100%, then you want to create a container within the body. This container can then be set as a flexbox to be centered within the body.
Without a set viewport height/width, the container or elements inside the body do not know any boundaries.
Example :
html, body { height: 100%; }
body { display: flex; justify-content: center; align-items: center; margin: auto; }
.container { /* add your container styles here */ }
Hope this helps!
Marked as helpful
0 - Bassey David Edet• 10
@Davidbassey01
Submitted
Cesar D.• 400@ThatDevDiaz
Posted
Hey it looks like you got a little bit of extra padding under the qr code. There's a few ways to fix this. You can add padding-bottom to the qr code separating the text below it a little bit further which will make it look like there's less empty space in 1 large block. I would open dev tools and take a look at which element is creating all that empty white space and adjust accordingly using margin/padding or maybe just shrinking the size of the entire container.
Also, I recommend downloading and importing/using local fonts because you never know if a font imported from a link will break. Links change all the time and if it's linked to a google site, any changes can affect the font in future imports. Keeping it locally ensures this won't happen because the font file will always be in the same place.
Good job on the HTML and keeping it simple. I'd probably change the H3 to an H1 and just change the font. Always good to have an H1 in a project file.
Good job!
Marked as helpful
0 - Chris Sutcliffe• 10
@testpilotuk
Submitted
As a beginner I would like to see a more elegant and correct solution for positioning and creating the correct amount of white space. This was my first challenge and I must admit I enjoyed it.
Cesar D.• 400@ThatDevDiaz
Posted
Hey, you pretty much nailed it.
You should use the font weight within the stylesheet so the text below the QR code is just a little bit closer to the original than what you have. I believe the fonts-weight was 400 and 700 on the provided sheet.
Your HTML has a lot of "links" in them when in reality you only needed to link your stylesheet. Everything else could've been done within the stylesheet like the font-family for example.
Setting your font size to a % instead of 16px as shown within the stylesheet was something I'm not 100% sure about why it was done, so I don't want to comment on that, but it is something I noticed.
I would avoid using "ID" and instead use classes more often. It's a good habit to have.
As for the QR code, you could've maybe specified margin-left and margin-right to get a more even spacing between the QR code and the container. There's other ways to do this too depending on the situation. Specifying left/right padding might have worked too. I would've played around with both and worked with whichever gave me the result I was looking for. Ideally, using flexbox and separating the design into 2 major components within 1 "container" might have made this easier.
You could have also added a "Border-shadow" to the container since it seems like you didn't have one when the original design did. This is something I just recently learned myself.
Nothing too major to fix that I see from my newbie eyes, just small things that might maybe help in the long run from what I've been learning.
Good job
Marked as helpful
0 - HoangTuanAnh• 390
@TuanAnh45468
Submitted
Hi there! I'm currently learning CSS and working on this small project to sharpen my skills. I'm not entirely confident about media queries yet, so if you have any suggestions for improvement, please feel free to provide feedback.
Cesar D.• 400@ThatDevDiaz
Posted
Hey, pretty close.
The stylesheet should be using colors and fonts provided by the designer (or in this case the challenge's design sheet). If you wanted to use your own colors and fonts you absolutely could, but it's probably good practice to use what's provided.
Not sure why there's so much JS or Tailwind incorporated in here for an HTML/CSS project. Maybe the notes specified you could use it for this challenge, and I didn't catch that.
Github is showing the original readme instead of the one that should've been replaced by your notes. Small detail but might help you down the line for bigger stuff with transparency.
Your HTML seems a little cluttered. Try looking at the project as 2 sections within the "container". The QR code itself, and the description. You have multiple paragraph elements when really you only needed 1 following an H1 element. This helps a lot when you're on the CSS sheet because there's less clutter to work through.
*Side note. Try to minimize styling your HTML in the HTML itself. You have a lot of font changes within the HTML which can be confusing for larger projects.
As for the Text below the QR code, you could've aligned the text to the center using text-align and that should've had the text looking more like the original. Also experiment with max-width within text elements so that you can force text to stay in a certain paremeter. If you're using flexbox, make sure you're centering the element within the flexbox because max-width will not work on it's own. The text will be glued to the left side of the flex box if you only use max-width which is why you also have to center it horizontally using jusify-content. You'll see what I mean if you give it a try.
Overall, you got most of it down, the biggest thing I noticed was the discrepancies in the design with the QR code surrounding color and the font. This is what somebody looking from the outside in will notice right away if they didn't have access to the code.
Good stuff!
Marked as helpful
1 - sameul84• 10
@sameul84
Submitted
Cesar D.• 400@ThatDevDiaz
Posted
Looks pretty good!
Some pointers from me that I can forward from my own project experience would be :
Instead of using H2, use H1 and just change the font. Every HTML page should have an H1 instead of skipping over it.
The Readme file notates that you should replace it with your own personal notes, so although you did upload 2 readme files, the one that shows on the github project is the original template. No biggie, just a minor detail.
The font family used is not the one on the design sheet, make sure you're using the provided designs
These are just things I know helped me. Obviously there might be more things that I didn't catch or somebody else might be able to help more with. It was interesting seeing your perspective on the CSS stylesheet.
Overall, small things that don't make or break a project. Good stuff!
0
