Skip to content

Add art interactivity#11

Open
Ramazanmak wants to merge 13 commits intomainfrom
feat/art-interactivity
Open

Add art interactivity#11
Ramazanmak wants to merge 13 commits intomainfrom
feat/art-interactivity

Conversation

@Ramazanmak
Copy link
Collaborator

No description provided.

@Ramazanmak Ramazanmak requested a review from mr150 February 1, 2026 11:24
@Ramazanmak Ramazanmak self-assigned this Feb 1, 2026
}
%>
<a href="<%= it.url %>" role="button"
<<%= it.tag %> <%= href %> role="button"
Copy link
Contributor

Choose a reason for hiding this comment

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

role="button" - зачем это?


<header class=" D-f Jc-sb Ai-c P2u;2u md_P1u;3u Bgc-$core800 -Blr4 Bcf-$blr Ps-f H-$headerH
W100p C-$headerC Zi20">
<header class=" D-f Jc-sb Ai-c P2u;2u md_P1u;3u Bgc-$core800 -Blr4 Bcf-$blr Ps-f H-$headerH W100p C-$headerC Zi20">
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<header class=" D-f Jc-sb Ai-c P2u;2u md_P1u;3u Bgc-$core800 -Blr4 Bcf-$blr Ps-f H-$headerH W100p C-$headerC Zi20">
<header class=" D-f Jc-sb Ai-c P2u;2u md_P1u;3u Bgc-$core800 -Blr4 Bcf Ps-f H-$headerH W100p C-$headerC Zi20">

Copy link
Contributor

Choose a reason for hiding this comment

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

Эти файлы должен eleventy генерировать во время сборки...

Copy link
Contributor

Choose a reason for hiding this comment

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

Зачем отдельный каталог? И название надо другое: типа art-panel

@@ -0,0 +1,53 @@
const hightlightClasses = "Bd1;s;$brand Bxsd1;1;10;2;$brand Zi19"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const hightlightClasses = "Bd1;s;$brand Bxsd1;1;10;2;$brand Zi19"
const hightlightСss = "Bd1;s;$brand Bxsd1;1;10;2;$brand Zi19"

connectedCallback() {
this.highlightTargetArt()
const artId = this.getAttribute('id');
console.log(artId);
Copy link
Contributor

Choose a reason for hiding this comment

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

Логи убираем

src/arts.ejs Outdated
variant: "light"
}) %>
</div>
<div id="<%= art.artId %>-link-button" class="<%= artsPageCSS.button %>" >
Copy link
Contributor

Choose a reason for hiding this comment

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

Зачем id кнопке?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Чтобы компонент мог обращаться к этой кнопке без привязки к тегу.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Чтобы обращаться к ней, не привязываясь к имени тега.

Copy link
Contributor

Choose a reason for hiding this comment

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

Но есть же классы для этих целей)

button.addEventListener('click', this.copyArtLink(artId))
}

copyArtLink = (artId) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Почему здесь id берется из замыкания, а не просто кладется в свойство класса?


copyArtLink = (artId) => {
return function() {
const span = this.querySelector('span')
Copy link
Contributor

Choose a reason for hiding this comment

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

Не надо завязываться на названия элементов

const span = this.querySelector('span')
let link = window.location.href
const idPosition = link.indexOf('#')
if (idPosition > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Это можно сильно проще написать, без if

let link = window.location.href
const idPosition = link.indexOf('#')
if (idPosition > 0) {
link.replace(
Copy link
Contributor

Choose a reason for hiding this comment

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

replace не меняет исходную строку

link += `#${artId}`
}
navigator.clipboard.writeText(link)
span.innerText = 'Copied!'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
span.innerText = 'Copied!'
span.textContent = 'Copied!'

}
}

highlightTargetArt(){
Copy link
Contributor

Choose a reason for hiding this comment

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

Посмотри инфу про псевдокласс :target

Copy link
Contributor

@mr150 mr150 left a comment

Choose a reason for hiding this comment

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

!

@Ramazanmak Ramazanmak force-pushed the feat/art-interactivity branch from 09b9422 to 52f084b Compare February 2, 2026 14:52

highlightTargetArt() {
hightlightCss.split(' ').forEach(c => {
this.classList.add(c)
Copy link
Contributor

Choose a reason for hiding this comment

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

В classList можно несколько элементов добавлять и удалять

connectedCallback() {
this.highlightTargetArt();
const button = this.querySelector(`#${this.artId}-link-button`);
button.addEventListener('click', () => this.copyArtLink(button))
Copy link
Contributor

Choose a reason for hiding this comment

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

Зачем здесь передается эта кнопка, если ее так же можно в поле компонента положить?

src/art.ejs Outdated
@@ -0,0 +1,8 @@
---
pagination:
Copy link
Contributor

Choose a reason for hiding this comment

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

Причем здесь пагинация? Надо сделать каталог с артами, где они будут лежать, и откуда берутся в галерею. При сборке он будет переименовываться в art-api и там они уже будут собранные для выкачивания

<% } else { %>
<div class="<%= css.contentContainer%>">
<span class="<%= css.span %>">
<span class="<%= css.span %>" data-name="button-text">
Copy link
Contributor

Choose a reason for hiding this comment

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

Зачем атрибут, если можно использовать class?

export class ArtPanel extends HTMLElement {
constructor() {
super();
this.artId = this.getAttribute('id')
Copy link
Contributor

Choose a reason for hiding this comment

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

Можно просто использовать this.id

.eleventy.js Outdated
export default async function(config){
config.addPlugin(ejsPlugin,{
_with: false,
localsName: 'it'
Copy link
Contributor

Choose a reason for hiding this comment

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

Что-то не так с форматированием

}

highlightTargetArt() {
this.classList.add('Bxsd1;1;10;2;$brand_t', 'Bd1;s;$brand_t', 'Zi19_t')
Copy link
Contributor

Choose a reason for hiding this comment

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

Зачем убирать эти утилиты из переменной, если можно просто ее деструктуризацию использовать?

@Ramazanmak Ramazanmak requested a review from mr150 February 3, 2026 16:15
connectedCallback() {
this.highlightTargetArt();
this.button = this.querySelector(`#${this.id}-link-button`);
this.button = this.querySelector(`.${this.id}-link-button`);
Copy link
Contributor

Choose a reason for hiding this comment

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

А зачем здесь вообще к классу id прицеплять?

@@ -1,3 +1,5 @@
const hightlightCss = "Bd1;s;$brand_t Bxsd1;1;10;2;$brand_t Zi19_t"
Copy link
Contributor

Choose a reason for hiding this comment

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

Если это используется только как массив, то может и сразу сделать массивом?

@@ -6,7 +8,7 @@ export class ArtPanel extends HTMLElement {

connectedCallback() {
this.highlightTargetArt();
Copy link
Contributor

@mr150 mr150 Feb 3, 2026

Choose a reason for hiding this comment

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

И вот что подумал. Наверное лучше без target утилиты оставить и просто в js проверять, есть ли id в hash и только тогда это вызывать. А то когда артов станет сильно больше, то много лишних операций в dom будет

@Ramazanmak Ramazanmak requested a review from mr150 February 4, 2026 02:58
src/arts.ejs Outdated
<div class="<%= artsPageCSS.card%> md_W50p">
<div class="<%= artsPageCSS.artWrapper%> <%= art.bgColor %> ">
<%- include(`./_includes/components/arts/${art.artId}.ejs`) %>
<art-panel class="D Ps -Ctx W100p H20gg -Ts @:hv_Tsd-$longTs md_W50p" id="<%= art.artId %>">
Copy link
Contributor

Choose a reason for hiding this comment

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

Зачем @:hv_Tsd-$longTs используй дефолтное

src/arts.ejs Outdated
const artsPageCSS = {
card:"W100p H20gg Tsd-$shortTs @:hv_Tsd-$longTs",
artWrapper:"-Sz100p Ov-h Plcc-c Plci-c D-f",
button: "W12gg lg_W9gg Mxw50u xl_Mxw55u H7u"
Copy link
Contributor

Choose a reason for hiding this comment

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

Уже не раз говорил: нельзя ограничивать элементы с текстом по высоте

@Ramazanmak Ramazanmak requested review from mr150 February 4, 2026 12:24
src/index.ejs Outdated
icon: "right-triangle"
}) %>
<div class="D-f Fld-r W100p Flw-w Jc-c Gap3u M3u;0 P0;5u Mnh7u">
<div class="W70p Mxw50u xl_Mxw55u As-str">
Copy link
Contributor

Choose a reason for hiding this comment

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

Зачем в нескольких местах надо добавлять этот div?

artLayer: "W100p Apcr1 Bdrd100p Ps-a"
}
%>
<div class="Ctnt-s Pt5u H100p Apcr1 Ctnn-canva Plci-c;c Plcc-c D-f M-a -ExplodingOrangeA#ee980d -ExplodingOrangeB#f8c829 -ExplodingBrownA#662800 -ExplodingBrownB#6f2810 -ExplodingBrownC#9c2a00 -ExplodingBrownD#934800 -ExplodingBrownE#9b3c18 -ExplodingBrownF#a24b0b -ExplodingBrownG#f7ae48 -ExplodingBrownH#d2a55a -ExplodingBrownI#efd3ac">
Copy link
Contributor

Choose a reason for hiding this comment

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

Pt5u на главной из-за этого арт уменьшился - не надо так

@Ramazanmak Ramazanmak requested a review from mr150 February 8, 2026 11:04
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