Anton Sukhadolets
@SuhodolecAAll comments
- @500EJSubmitted over 2 years ago@SuhodolecAPosted over 2 years ago
Hello! Well done!
In my opinion, you could improve your solution using the next steps:
- input[type="number"] add next properties:
- "cursor: pointer", shows mouse hover.
- hover effect, using pseudo-class ":hover", ":focuse" like in layout.
- add "border" with transparent color and by hover change color
- add "transition" property for smooth animation
For example:
input[type="number"] {
background-color: hsl(189, 41%, 97%); border-radius: 5px; font-weight: 700; height: 30px; margin: 0 auto; -moz-appearance: textfield; padding: 20px 10px; text-align: right; width: 100%; cursor: pointer; border: 1px solid transparent; transition: border .3s linear;
}
input[type="number"]:hover { border-color: hsl(172, 67%, 45%); }
input.bill:focus { border-color: hsl(172, 67%, 45%); outline: none; }
Looks better!
- Your calculator twitches when calculating results. It happens because tip-price divs change the width. You can fix this add fixing "width" and "text-align: right" but it is better for displays using "inputs type text" with disabling property. I show with divs but better use "inputs".
For example:
.price { color: hsl(172, 67%, 45%); font-weight: 700; min-width: 220px; text-align: right; }
And you should restrict max input value in js it exists in all calculators.
Have a nice day and good luck!
Marked as helpful1 - @martinnovak22Submitted over 2 years ago
Any comment or advice is helpful. Have a nice day.
@SuhodolecAPosted over 2 years agoHello! Not a bad solution! Also, I have some tips for improving your result:
1)For left-side p, use "font-weight" property and another "color". This will make your solution look more like a layout. And reduce padding till 5px;
For example:
.left-side p {
padding: 0 0 5px; margin-top: 20px; font-weight: 700; color: hsl(186, 14%, 43%);
}
2)When I click .input-num all things inside twitch. It happens so you change the input side by adding a border. You can fix this, but you need to change your approach. Add a border to this input in general styles and set "color": transparent and then change only color but not border. Then use :hover and :focus pseudo-classes for triggering. And add "transition property for smooth animation", add this property for all elements where you want smooth animation. Also add "cursor":pointer.
For example:
.input-num {
border-radius: 5px; min-height: 30px; text-align: right; font-family: "Space Mono", sans-serif; background-color: hsl(185, 41%, 84%); border: 1px solid transparent; transition: all 0.3s linear; cursor: pointer; padding: 5px;
}
.input-num:hover {
border-color: hsl(172, 67%, 45%);
}
.input-num:focus {
border-color: hsl(172, 67%, 45%);
}
- <input class="input-num" type="number" id="bill-num" value="0" onchange="getBill(this)" />
Replace value = 0 on placeholder="0" and add style using pseudo-class :placeholder;
For example:
<input class="input-num" type="number" id="bill-num" placeholder="0" onchange="getBill(this)" />4)You can hide arrows in .input-num using next code:
.input-num::-webkit-inner-spin-button { -webkit-appearance: none; }
That is only part of the changes but if you use this your solution will be better!
You are doing well, have a good day!
0 - @ThiagovascSubmitted almost 3 years ago@SuhodolecAPosted almost 3 years ago
Hello! Beautiful work! With a few changes, you could improve your solution.
1)You have a little gap between "main-picture" and "container". You can fix this using:
.main-picture { display: block; or vertical-align: middle; }
- The main section has the wrong background color. Fix:
main { background-color: white; max-width: 400px; border-radius: 1rem 1rem 0 0; }
- plan-info section, use the following properties to fix some differences between design and your solution: "border-radius", "padding", remove "justify-content: space-around" this property For example:
.plan-info { display: flex; align-items: center; width: 100%; background-color: var(--VeryPaleBlue); margin: 2rem; border-radius: 10px; padding: 10px; }
10px values it's approximate because i don't have that layout now, use correct values.
Next, .plan-info img use here margin-right For example:
plan-info img { justify-content: flex-start; max-width: 100%; margin-right: 10px; } 10px values it's approximate because i don't have that layout now, use correct values.
And for plan-info a use "mragin-left: auto"
.plan-info a { color: var(--BrightBlue); font-weight: 500; margin-left: auto; } After these fixes, in my opinion, your solution will be exactly the same as the layout!
Good luck!
Marked as helpful1 - @tcaturani-gossSubmitted almost 3 years ago@SuhodolecAPosted almost 3 years ago
Hello! Nice work! You could improve your solution using transition property for interactive elements. For example: .menu-link { margin-left: 0.9rem; cursor: pointer; transition: color 0.3s linear; } .menu-link:hover { color: hsl(0, 0%, 100%); } It gives you smooth animation.
Also, you can add cursor: pointer property for .regular-card element. And you could resolve problems with cards(parent background is visible when resizing page) using :
- For parent overflow: hidden;
- For child height: 100% Like this: .regular-card { border: 1px solid var(--Very-dark-blue); border-radius: 0.8125rem; background-repeat: no-repeat; background-position: 93% -0.7rem; cursor: pointer; overflow: hidden; }
.data-card { position: relative; background-color: hsl(235, 46%, 20%); border-radius: 0.8125rem; padding: 1.75rem 1.5rem 1.8rem 1.5rem; margin-top: 2.4375rem; height: 100%; }
Good luck!
Marked as helpful1 - @AngelG-JAPYSubmitted almost 3 years ago
This practice was difficult for me. I think it could be better, but can you give me some recommendations. Thanks.
@SuhodolecAPosted almost 3 years agoGood job! You can resolve problems with social icons using flexboxes. For example: .fab, .far { font-weight: 400; width: 40px; height: 40px; display: inline-flex; align-items: center; justify-content: center; } And after that change font-size icons for a better view. Try to use transition property for smooth animation of your interactive elements like buttons and social icons.
Good luck!
Marked as helpful0 - @Abdullahi-1998Submitted almost 3 years ago
Feedback would be great 👍.
@SuhodolecAPosted almost 3 years agoHello! Beautiful work! I've found some things that could improve your solution! 1.) In logo-section, wrap your img tag in <a> tag.(<a><img src="images/logo.svg" alt="Logo for Huddle"></a>). I think it would be better because most pages has in his logo has link for themselves. 2.) Use transition property for smooth changing intaractive elemets.(button{ transition: all 0.3s linear }). 3.) Use that constrution for social links:
<ul> <li><a>facebook</a></li> <li><a>twitter</a></li> <li><a>instagram</a></li> </ul> Because you have several related links that is why it is a list. 4.) Use this construction for social link, that helps you centered icons. .fab { display: flex; align-items: center; justify-content: center; } And transition property for smooth animations.Good luck!
Marked as helpful0 - @jtrivelSubmitted almost 3 years ago@SuhodolecAPosted almost 3 years ago
Hi! Looks great! Really good work! But one little clarification from my side. I think the interactive elements need to be links(<a></a> tags). It means when you click for the headline or main image or creator name you should move to the article, creator profile, etc.
Good luck!
Marked as helpful1 - @anoshaahmedSubmitted almost 3 years ago
I would appreciate opinions on the organization of the HTML and CSS code. Also, if there is a more efficient way to do something than what I chose to do.
- @vonaserSubmitted almost 3 years ago
I am not sure that I applied responsiveness to this design correctly. So, what do you suggest for improving this design's responsiveness?
@SuhodolecAPosted almost 3 years agoHey! Great job! But I think you can do better! I noticed there are empty alt attributes in your html code. It might be better to fill it in. I also recommend you to add transition property in order to add smoothness for interactive elements. And last, сhange fixed width and height of your card, it must be responsive.
Marked as helpful0