@ericsalvi
Posted
Hey Roy,
This looks really good for someone being self-taught for a month. Keep up the great work.
One thing that did stand out to me from a visual side of things is that the color styles for the background and fonts do not match the solution/design. I would take another look at those things to try and get them as closely matched as possible.
The responsiveness works so that is great. I did not test on different browsers so you may have some incompatibilities on older browsers. I'd run your CSS through https://autoprefixer.github.io/ and then use the generated code for your CSS. It adds all the browser compatible flex stuff and other things needed to make sure it works for almost all viewers.
I think you could have done one extra thing to set the hover, focus, and active states on the button. Play what that. You have a license for creativity there.
Lastly, the code itself looks clean and pretty straightforward. I think for future solutions you should check out semantic HTML markup https://developer.mozilla.org/en-US/docs/Glossary/Semantics and read about BEM naming conventions. Just some tips.
Keep up the momentum!
@ericsalvi Thank you very much for your kind and detailed feedback.
I will look into the color styles not matching. I don't know what I did to change them.
I tested in Chrome and Firefox. Would that be enough or is it better to test in all browsers?
I tried to implement your helpful advice regarding the CSS autoprefixer, button states and markup semantics in the next challenge I just uploaded: https://www.frontendmentor.io/solutions/3column-preview-card-component-with-html-and-css-VWvdp-3y_
Thank you very much for the tips. I really appreciate it.