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

Npm #3

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Npm #3

wants to merge 4 commits into from

Conversation

Alexandra-Tkachenko
Copy link
Collaborator

No description provided.

--color-shadow: #c4c4c4;
}

@media (prefers-color-scheme: dark) {
Copy link

@Leikam Leikam Jan 2, 2022

Choose a reason for hiding this comment

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

по-хорошему надо еще дать пользователю возможность выбирать тему. Но и твой подход через системные настройки неправильно обходить вниманием и, как демонстрация этого достаточно. Хорошо, что узнала про него.

const rollup = require('rollup');
const image = require('gulp-image');

const assetsPath = 'src/assets/*.{png, jpeg, psd, bmp, gif, tiff, icon, ico}';
Copy link

Choose a reason for hiding this comment

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

конфиги с путями и константами можно вынести в отд файл. для удобства

const bundle = await rollup.rollup(rollupConfig);

bundle.write({
format: 'esm',
Copy link

Choose a reason for hiding this comment

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

для справки, частая задача сборщика – подготовить код для браузеров, которые не поддерживают уровень языка на котором код пишется в сорцах. В таких случаях тут можно использовать iife, что бы развернуть импорты в один большой супермодуль. Не так красиво, но работает везде.

<h3 class="about__annotation">Web Designer</h3>
</div>
<section class="social-network">
<img class="social-network__telegram" src="assets/telegram_logo.png" alt="telegram">
Copy link

Choose a reason for hiding this comment

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

тут можно улучшить, если сделать класс + модификатор, что-то нейтральное для общих стилей всех элементов списка вроде:
social-network__item
добавить второй класс-модификатор для уник. стилей, как ссылки на иконку, брендовые цвета и тп.
social-network__item_telegram

@@ -0,0 +1,21 @@
@import "reset.scss";
Copy link

@Leikam Leikam Jan 2, 2022

Choose a reason for hiding this comment

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

хорошо разбила на компоненты 👍

@@ -0,0 +1,5 @@
.resume__copyrights {
Copy link

Choose a reason for hiding this comment

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

Иногда сложный компонент тоже бьется на несколько файлов, но этот пока небольшой, его омжно хранить вместе с resume для удобства использования.

Copy link

@Leikam Leikam left a comment

Choose a reason for hiding this comment

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

Отличная работа, все хорошо разбито по компонентам, иногда, даже слишком подробно :) Например resume и resume__information скорее всего лучше хранить вместе, как неделимый компонент – так будет проще видеть весь компонент, его вариации и историю изменений.

К сборке особых замечаний нет, все на месте.

Оценка 10/10

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.

2 participants