search

freeCodeCamp / CurriculumExpansion

Creative Commons Attribution Share Alike 4.0 International
313 stars 105 forks source link

feat: add styling forms lab #467

Closed ihechikara closed 1 month ago

ihechikara commented 2 months ago

Checklist:

Closes #396

ihechikara commented 2 months ago

It looks good 👍 I left a few comments.

I have a doubt about using CSS property names with and without backticks. I'm okay with not surrounding property names for background color, border, or font size. But sometimes it seems weird to me. For example, border radius, or padding.

I'm saying this, mainly because there's no accurate translation of those, and I think it might lead to inaccuracies if translated. Of course this may not be true for other languages.

@Dario-DC, I decided to put most of the property names within backticks. Based on your suggestion, I think that'll make translating them more straightforward. Let me know what you think.

ihechikara commented 1 month ago

This looks good @ihechikara 👍 I would be fine it as-is, but here's some suggestions...

Make some of the text of the user stories more flexible - so users don't have to use exact values. For instance, instead of:

A `margin-bottom` of `10px`., Maybe just A margin on the bottom. So people can use a value they want.

It's not necessary to change though, just mentioning it. You have several user stories that do allow for some flexible.

I also like #4: Each `input` and `textarea` element should have their corresponding `label` element with a `for` attribute set to the element `id`. I feel like this makes people think about what they have to do more, instead of just copying what they're told to do. For example one says to add A `padding` of `10px`. - It's fine to have some of these, but it feels like users don't really have to think about that. Again, just mention it - maybe something to keep in mind for future labs.

I would be fine getting this in if you are @ihechikara 👍 Feel free to merge it, or make adjustments if you think some of those should be changed.

Thanks for the suggestions, @moT01! I've made the user stories more flexible. Just wondering - should I do the same with #3 in the user stories? Let me know what you think.

moT01 commented 1 month ago

LGTM 👍