Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

completed the task UiStretch #11

Merged
merged 2 commits into from
Oct 13, 2024
Merged

Conversation

alexmenshikov
Copy link

Правильное ли такое решение?

@jsru-1
Copy link
Contributor

jsru-1 commented Oct 7, 2024

Добавляю преподавателя (@ShGKme) для код-ревью.

@jsru-1 jsru-1 requested a review from ShGKme October 7, 2024 07:22
Comment on lines 15 to 17
:deep(.stretch-container > img),
:deep(.stretch-container > video),
:deep(.stretch-container > picture) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Такое решение работает. Но, если в содержимое компонента даже где-то глубоко-глубоко случайно тоже будет элемент попадающий под .stretch-container > img - на него эти стили тоже подействуют.

Нужно немного поправить селектор так, чтобы он действовал только и только на img/video являющиеся непосредственно дочерними для дива этого компонента.

@jsru-1
Copy link
Contributor

jsru-1 commented Oct 9, 2024

Добавляю преподавателя (@ShGKme) для код-ревью.

@jsru-1 jsru-1 requested a review from ShGKme October 9, 2024 14:26
@alexmenshikov
Copy link
Author

Если :deep применить только к самим img/video, а не полностью всё оборачивать

Comment on lines +15 to +17
.stretch-container > :deep(img),
.stretch-container > :deep(video),
.stretch-container > :deep(picture) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Хотя deep с > работает точно также, лучше здесь использовать :slotted. Семантически - стилизуется именно содержимое слота.

@jsru-1 jsru-1 merged commit 70d7435 into js-tasks-ru:master Oct 13, 2024
1 check passed
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