Skip to content

feat(components/button): prizmButton without text should appear like [prizmIconButton] #2361#2429

Closed
ickisIckis wants to merge 7 commits intomainfrom
feat/button-icon-2361
Closed

feat(components/button): prizmButton without text should appear like [prizmIconButton] #2361#2429
ickisIckis wants to merge 7 commits intomainfrom
feat/button-icon-2361

Conversation

@ickisIckis
Copy link
Copy Markdown
Contributor

@ickisIckis ickisIckis commented Mar 21, 2025

feat(components/button): prizmButton without text should appear like [prizmIconButton] #2361 special thanks to @AleksandrSibiakov for help with solution

fix(components/switcher): switcher with only icon should not render extra button #2412

refactor(helpers/has-value): robust content detection added

resolved #2361
resolved #2412

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 21, 2025

Visit the preview URL for this PR (updated for commit fa13ca4):

https://prizm-v4--pr2429-feat-button-icon-236-2h9w4tuo.web.app

(expires Tue, 01 Apr 2025 13:17:10 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 7c62ed8dbabf5e2d6b2084ca9e107cc206d30dbd

@bumalai
Copy link
Copy Markdown
Contributor

bumalai commented Mar 21, 2025

prizmIconButton можно депрекеитить? така как функционал одинаковый?

Comment thread libs/components/src/lib/components/button/button.component.less Outdated
@ickisIckis
Copy link
Copy Markdown
Contributor Author

prizmIconButton можно депрекеитить? така как функционал одинаковый?

тоже появилась такая мысль, в дизайне у нас есть две сущности сейчас. надо проговорить это с @PrizmDS

Copy link
Copy Markdown
Contributor

@bumalai bumalai left a comment

Choose a reason for hiding this comment

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

lgtm

@alexhawkins94
Copy link
Copy Markdown

В live demo если у кнопки не задан текст внутри кнопки и задана иконка, то присутствует лишний отступ справа от иконки - кнопка становится размеом 56х44
chrome_vNx64wwcO5

@ickisIckis ickisIckis force-pushed the feat/button-icon-2361 branch from 5859b32 to ebf9a0d Compare March 27, 2025 12:59
@ickisIckis ickisIckis modified the milestones: 5.9.0, 5.10.0 Mar 28, 2025
Copy link
Copy Markdown
Contributor

@AleksandrSibiakov AleksandrSibiakov left a comment

Choose a reason for hiding this comment

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

Думаю проверять все поддерево через mutationobserver (в контексте кнопок) - перебор. Достаточно чекать только пробельные символы в TextNode дочернем (покрыть недостающий :empty функционал). А то потом не сможем отказаться от этой имплементации, когда выйдет CSS4, т.к. это будет Breaking Change

Comment thread libs/components/src/lib/components/button/button.component.less
const el = node as HTMLElement;

const style = window.getComputedStyle(el);
if (style.display === 'none' || style.visibility === 'hidden') {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

visibility: hidden резервирует место в dom - может не стоит включать в проверку?

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

Projects

None yet

4 participants