Skip to content

Conversation

denisx
Copy link
Contributor

@denisx denisx commented Sep 20, 2025

Markdown : cплит внутреннего компонента десктоп / мобайл

Copy link

changeset-bot bot commented Sep 20, 2025

🦋 Changeset detected

Latest commit: bcb2c9a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@alfalab/core-components Patch
@alfalab/core-components-markdown Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@denisx denisx changed the title Feat/aotechtask 6731 02 Feat/markdown: split inner component desktop / mobile Sep 20, 2025
@coveralls
Copy link

coveralls commented Sep 20, 2025

Pull Request Test Coverage Report for Build 17916997489

Details

  • 14 of 14 (100.0%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.009%) to 79.746%

Totals Coverage Status
Change from base Build 17868060226: 0.009%
Covered Lines: 9671
Relevant Lines: 11363

💛 - Coveralls

Copy link
Contributor

Demo build

https://core-ds.github.io/core-components/1891

@denisx denisx marked this pull request as ready for review September 21, 2025 08:24
@fulcanellee
Copy link
Collaborator

В кодовой базе библиотеки любой base компонент подразумевается в единственном экземпляре. Текущий PR просто размножил этот базовый компонент, ради того, чтобы разделить типографику. Текущие решение считаю избыточным.

@denisx
Copy link
Contributor Author

denisx commented Sep 22, 2025

В кодовой базе библиотеки любой base компонент подразумевается в единственном экземпляре. Текущий PR просто размножил этот базовый компонент, ради того, чтобы разделить типографику. Текущие решение считаю избыточным.

Если типографика не base, то как зависимые от неё компоненты могут быть монолитны?

Да и компоненты остался на месте. Просто у него добавился пропс. Пропсы в любом случае нужно передавать. Если ему нужен responsive импорт, давай доделаю, чтобы был base resposive (я просто думал, что импорты компонентов у нас только из корня пакетов, и внутряшки не требуют респонсива)

@fulcanellee
Copy link
Collaborator

Нужную типографику ты мог передать тут, не создавая BaseMarkdownDesktop и BaseMarkdownMobile

image

@dHIM24
Copy link
Contributor

dHIM24 commented Sep 22, 2025

Поддерживаю позицию @fulcanellee , как будто практической пользы доработка не несет

@denisx
Copy link
Contributor Author

denisx commented Sep 22, 2025

да, просмотрел этот момент. упростил пр

Copy link
Collaborator

@fulcanellee fulcanellee left a comment

Choose a reason for hiding this comment

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

Отлично. Еще измени базовую ветку PR'a. next предназначена для мажорки. Также отметил пару моментов ниже.

@fulcanellee fulcanellee changed the base branch from next to master September 22, 2025 09:51
@fulcanellee fulcanellee changed the base branch from master to next September 22, 2025 09:51
@fulcanellee
Copy link
Collaborator

Не забудь next на master поменять. Нужно будет зачистить ветку до состояния мастера, а накатить изменения git патчем

@denisx
Copy link
Contributor Author

denisx commented Sep 22, 2025

@fulcanellee запутался. ставлю деприкейт на platform, и ветку на мастер (вар.1)
или оставляю как мажорку, и в next? (вар.2)

@fulcanellee
Copy link
Collaborator

fulcanellee commented Sep 22, 2025

@fulcanellee запутался. ставлю деприкейт на platform, и ветку на мастер (вар.1) или оставляю как мажорку, и в next? (вар.2)

я удалил тот коммент. сделал коммит. Делаем omit Title и выпускаем как патч от мастера

@denisx denisx changed the base branch from next to master September 22, 2025 12:20
@denisx denisx force-pushed the feat/AOTECHTASK-6731_02 branch from 4a9a8ec to b5036c7 Compare September 22, 2025 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants