luking
@alululululuerAll comments
- @zaktwo
- @bkpecho@alululululuer
Hello! Here are some thoughts in my view.
- Aviod any horizontal scrollbar. Your media query min-width should set at least 750px, because your two parts width of this component seted to 375px and your min-width seted to 700px which smaller than the card width.
- Your BEM rules seem a little confused👀. I'm also learn BEM and I do not well too. "--"should be used as modifier. I think it maybe look better:
.card {} .card__result {} .card__summary {} .summary__wrapper--reaction {} .summary__wrapper--memory {}
Just from my perspective, hope it's helpful to you.
Marked as helpful - @avigithubb@alululululuer
Hello! Congratulations on starting your first frontend journey. Give you some advice from my perspective.
-
HTML
- put your .Outside-box into <main>
- put your .attribution into <footer>
- use <h1> instead of <p> for the text "Improve your..." as the accessibility report said that "Page should contain a level-one heading"
-
CSS
- remove width and height and margin of .Outside-box
- add max-width to .Outside-box
- to center the .Outside-box in the body you can use flex or grid layout to instead.
- for the beginner, I suggest you can use flex layout first.
- remember to add a height to the body at first.
- for the beginner, I suggest you can use flex layout first.
html,body { height: 100%; } body { display: flex; flex-direction: column; justify-content: center; align-items: center; }
- remove width and margin to the <img>, to make <img> fit the .My-img you can do the bottom. Look at the img reset
img { display: block; max-width: 100%; }
- always follow the style-guide as you can, your second-para's color is poor visibility.👀
Hope it's helpful to you and happy coding.
-
- @Alt08@alululululuer
Hello! For the hover state, you can use pseudo element "::before" or "::after" to implement.
- add background-color and background-image to the pseudo element
- remeber to add postion relative to section.container__img
- background-color should use rgba() or hsla() to add transparency then you can see "section.container__img" through the color.
- set the pseudo element's display value is "none" default state and "block" when hovering.
section.container__img { position: relative; } section.container__img::before { content: ""; position: absolute; top: 0; bottom: 0; left: 0; right: 0; background-color: hsla(178, 100%, 50%,0.5); background-image: url(./images/icon-view.svg); background-position: center; background-repeat: no-repeat; display: none; } section.container__img:hover::before { display: block; }
- some other issus after the you done obove
- remove the img element in the section.container__img and use background-image instead which is better to control the image size.
- give a height to the section.container__img
Hope it's helpful to you.
Marked as helpful - add background-color and background-image to the pseudo element