Skip to content

Add task solution#390

Open
Mykyta-snacj wants to merge 23 commits into
mate-academy:masterfrom
Mykyta-snacj:develop
Open

Add task solution#390
Mykyta-snacj wants to merge 23 commits into
mate-academy:masterfrom
Mykyta-snacj:develop

Conversation

@Mykyta-snacj

Copy link
Copy Markdown

DEMO LINK
Please check my mobile version and give some advices. Please also check my BEM. I don't know how to make sliders, so my shop section without slider.

@natalia-klonowska natalia-klonowska left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work :D fix BEM mistakes that I mark in comments and you can try to make sliders using e.g. input radio and transform translate (like here). Also you forgot about underline for all learn more elements

Comment thread src/index.html Outdated

<a
href="#shop"
class="button--type1 header__button--type1"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

button--type1 it's modifier but it's used like block
(https://mate-academy.github.io/fe-program/css/typical-bem-mistakes-en#using-a-modifier-without-the-belonging-class)

Suggested change
class="button--type1 header__button--type1"
class="button button-type1 header__button-type1"

or

Suggested change
class="button--type1 header__button--type1"
class="button button--type1 header__button"

or

Suggested change
class="button--type1 header__button--type1"
class=""header__button header__button--type1"

Comment thread src/index.html Outdated
<textarea
placeholder="Your Message"
id="message"
class="form__textarea form__input"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why it has two element classes? change one of them into modifier instead

Comment thread src/index.html Outdated

<a
href="#address"
class="button--type2 section__button--type2"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread src/index.html Outdated

<a
href="#address"
class="button--type2 section__button--type2"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread src/index.html Outdated

<a
href="#contact-us"
class="section__button--type1 button--type1 section__contact-button"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread src/index.html Outdated

<a
href="#address"
class="button--type2 section__button--type2"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Mykyta-snacj

Copy link
Copy Markdown
Author

DEMO LINK
Please check my tablet version and give some advices. Please also check my BEM.
please pay attenton for blocks: section, slider, shop, shop-card.

@danon321 danon321 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Teraz desktop oszalał

image

image
image

@Mykyta-snacj

Copy link
Copy Markdown
Author

Important!!!
I know that desktop is not ready. Check my tablet version. And pay Attention for blocks i wrote before
Thanks

Comment thread src/styles/blocks/button.scss Outdated
@use '../utils/mixins' as *;

.button {
&--type1 {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tutaj jedynie moze mogłabys uzyć lepszego nazwenictwa. Type1 mało mówi.
Patrz jak to rozwiązali np w bibliotece komponentów MUI
<Button variant="text">Text</Button> <Button variant="contained">Contained</Button> <Button variant="outlined">Outlined</Button>
https://mui.com/material-ui/react-button/

@Mykyta-snacj

Copy link
Copy Markdown
Author

DEMO LINK
Add task solution

@danon321 danon321 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks GREAT!!!! :))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants