That project was very nice project, I build it with tailwindcss and vanella js for the function so i need your feedback to fixe my bugs or i don't now 🤷♀️🤷♀️🤷♂️🤷♂️ to manage my code So welcom your feedback
Hi @aminetakdenti congrats on your solution, looks great.
Testing the form I noticed that, when you send it with errors and then you solve the errors there is no visual feedback, the error state remains, would be a good improvement to remove the error state when the form is OK.
I saw your javaScript and It looks very clean and compact. Something that I would recommend is to turn querySelector('div > input') into querySelector('input') is simpler and it does the same thing.
Also, I think you should try using the FormData object this is very handy when working with form and different inputs.
⭐️ Here is a cool trick you can do with this:
const form = document.querySelector('form')
// 👇Here you get an object where the keys are the input name, and the value is the input's value.
const formData = Object.fromEntries(new FormData(form))
I appreciate any feedback
Hello @Devibtissam your solution looks great.
I liked the animation on the title, and the design is very accurate. Something that I noticed was that email validation shows the same message for every error on the input, It would be nice if there were more specific error messages to provide feedback to the user.
For example:
If input is empty --> "Error the input cannot be empty"
If input is missing "@" --> "Error the input must have '@'"
I had(have) serious issues with styling the mobile view product image for a really long time now, so I went on to use the desktop view image; in simpler words, I don't know how to use different images when making a site responsive; I'd really appreciate any help on this.
Hello @Canice10, your solutions looks great. congrats 🎉
I saw that you have some problems with the mobile picture. There are different was to solve this problem. I can think of two right now that could be helpful for you.
<picture> tag, this allows you to render an image depending on the screen size or "breakpoint".
Here you have an example on how to use it, you set a <source> with a media(screen size) then the srcset(the image to show on this screen size)<picture>
<source media="(min-width:1300px)" srcset="./large.jpg1" />
<source media="(min-width:1000px)" srcset="./medium.jpg"/>
<img src="./img.jpg" alt="Image Description">
</picture>
background-image, this is another approach, you could set a media queries where it sets a different url base on the screen size..image-container {
background-image: url('small.jpg');
}
media(min-width: 800px) {
.image-container {
background-image: url('medium.jpg');
}
}
The only problem with this last solution is that you would have to set the size of the image container by hand to be able to see the image, and the image would not be accessible by screen readers, so be aware of this.
Hello @awsmPuff your solution looks very nice and feels smooth. The only thing that I noticed was that you set a fixed width with pixels and then you change it with a media query.
You could simplify this and remove the media query, the only thing you need to add is max-width and with this your card will only grow until it reaches the max-width value, and I will behave like it does with the media query.
Example:
.card {
width: 70%;
max-width: 350px;
}
Hope To Give Me any Suggestions to Improve myself
Hello @abdelrhmanKh congrats your solutions looks great.
I have some suggestions, first I noticed a rare jump between the content and when the fetch loads the real content, maybe you could do skeleton loader to avoid this.
Also, I checkout the main.js and you can change all the let for const because you are not reassigning those variables.
//Current
let advice = document.querySelector(".advice");
let adviceId = document.querySelector(".advice-Id");
let btn = document.querySelector(".get-advice");
//New
const advice = document.querySelector(".advice");
const adviceId = document.querySelector(".advice-Id");
const btn = document.querySelector(".get-advice");
Apart from a bit of refactoring is there any thing you would've done better, simpler, or neater here?
Hello @NiklausRupail, congrats on your solution, looks great.
I don't know too much about tailwind yet, but I notice some rare behavior with the card's width and the breakpoints, at some point the width between mobile and desktop the width goes big -> small -> big.
This challenge is simple and you don't need to set breakpoints, you can set a max-width on the card and mx-auto and that should do the trick for the card sizing.
It is done. This is my second challenge, and this time I felt less nervous. I must say that although the challenge did not require JavaScript I added a few lines of code to make the payment plan section change every time you click on the change button, that's the reason why it starts from the monthly plan and not yearly.
Any feedback is welcome!
Hello Ricardo, your solutions looks great and the added feature with JavaScript is great.
One thing that I noticed is that you used a border on the "change" link, this is not big deal but if you pay attention de text does a little jump when you hover in and out.
There are different ways to approach this, but in this case I think the intended version was to simulate a default <a> style with the underline, you can do this with text-decoration: underline.
Also another way to fix the jumping would be using outline instead of border.
I don't have the exact font sizes so judge the design only pls
Hello @omarsaleh11 don't worry about the fonts your solution looks very nice, but if you want the actual size, in the starter files in style-guide.md there is a font-size recommended.
Testing your solution I found that there is no padding in the grid__desc is important to set some padding to give the content some space to breathe. This is more clear when the width of the screen is around 748px, the content is alongside the border because there is no padding.
Hello @PabloRodrz your solution looks very great, the loading part is a nice touch. Doing some testing I noticed that the "advices" div has a height and width set with percentage(%). This could bring some issues. I suggest that you add a "max-width" along side the width to handle better the maximum horizontal size. And with the height, is better is you replace the height with min-height, because if the content inside is larger the height that you set the content won't be shown
Please support my studies here and on github, help me to get better, and i will make the same, let's make a healthy network, helping each others!!
Hello @Badarando I saw your solutions and It looks very nice. I was testing it and noticed that when you don't fill out the inputs the errors pop up, but the email input is filled with a placeholder text and the other ones are empty.
This is an accessibility issue for the users, this could be easily fixed by including a label with the input title, and with the placeholder property you add an example text in the input.
It was easy. I welcome your constructive criticism.
Hello @ramin47, your solution has the image broken, I saw your repo and the problem is that you don't have the image in the repo, you have to upload it in there or in another public source in other to place a working link in the image tag <img src="./images/img.jpg" />
Hy guys! I'm proud that this challenge I could work more easily than the first one I did, with a result more alike with the picture than yesterday. Fell free to show me what I can improve in my code.
Hello Gustavo, your solution link is not redirecting to the page, is redirecting to Vercel itself, you should check this out in order to see your working solution.
