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

tech(Flex): add support flex with gap #8212

Merged
merged 12 commits into from
Feb 3, 2025

Conversation

EldarMuhamethanov
Copy link
Contributor

@EldarMuhamethanov EldarMuhamethanov commented Jan 30, 2025

  • e2e-тесты

Описание

На данный момент мы не можем использовать свойство gap в flex контейнере - наша браузерная поддержка не покрывает поддержку этого свойства совместно с display: flex. Поэтому в компоненте Flex для реализации gap используются хаки с margin-ами. Хотелось бы использовать свойство gap как основную реализацию, и если она не поддерживается, то использовать фолбэк с текущей реализацией. Нужно придумать как это можно сделать используя только css.

Исследования

Первым делом попробовал использовать @support (gap: 1px) для проверки поддержки gap. Как оказалось этот способ не работает, так как само свойство появилось раньше. А его поддержка в flex контейнере появилась позже

Нашел issue на github в котором обсуждали эту проблему. В одном из комментариев пользователь предложил следующий способ:
Можно использовать @support (inset: 0), так как его браузерная поддержка максимально близка к поддержке gap + flex
Поддержка gap + flex
image
Поддержка inset
image

Из этого можно сделать вывод: @support (inset: 0) не будет срабатывать на все браузеры, которые не поддерживают flex + gap. Но также и не будет срабатывать для нескольких версий браузеров, где gap уже поддерживается, а именно: Chrome 84 - 86, Edge 84 - 86, Firefox 63 - 65, Opera 70 - 72.

Решение есть, осталось только определить плюсы и минусы данной доработки:

Плюс и минусы

+ У большинства пользователей будет использоваться нативное решение, у которого не будет багов, которые есть у фолбек решения, такие как вложенные Flex и тд.
- Разработчикам придется поддерживать в будущем(пока полностью не откажемся от фолбека) два подхода. Но я бы не сказал, что это будет проблемно, учитывая, что кода в целом то немного. Плюс небольшая вероятность, что в компонент Flex будет добавляться много функционала.

Немного статистики

Есть страничка со статистикой использования браузеров и их версий на caniuse. Исходя из нее можно сделать вывод, что очень мало пользователей пользуется браузерами, которые еще не поддерживают gap

Еще сайт со статистикой по браузерам

Кейсы, которые будут работать в новой реализации

Если прокинуть в children один Fragment, в котором будет несколько children
Реализация через gap:
image
В fallback реализации не будет выставлен:
image

Это происходит из-за этого кода:
image

Данный код правит баг из #7463. Можно, конечно, рекурсивно рассчитывать настоящее количество child-ов, но скорее всего это может ударить по производительности

В новой реализации обоих этих багов не будет

Изменения

  • Добавил использование gap если поддерживается свойство inset
  • Поправил код, чтобы размеры gap рассчитывались всегда, даже, когда 1 children. Это исправляет проблему, когда в children прокинут Fragment с множеством child-ов. Правда это исправляет проблему только для нового решения через gap
  • Доработал storybook компонента для более удобной работы с Flex

Release notes

Copy link
Contributor

github-actions bot commented Jan 30, 2025

size-limit report 📦

Path Size
JS 394.44 KB (-0.01% 🔽)
JS (gzip) 119.55 KB (-0.01% 🔽)
JS (brotli) 98.48 KB (+0.12% 🔺)
JS import Div (tree shaking) 1.56 KB (0%)
CSS 347.49 KB (+0.04% 🔺)
CSS (gzip) 43.02 KB (+0.06% 🔺)
CSS (brotli) 34.28 KB (-0.05% 🔽)

Copy link

codesandbox-ci bot commented Jan 30, 2025

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Copy link
Contributor

github-actions bot commented Jan 30, 2025

e2e tests

Playwright Report

Copy link
Contributor

github-actions bot commented Jan 30, 2025

👀 Docs deployed

Commit bd3505e

Copy link

codecov bot commented Jan 30, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.51%. Comparing base (b44e6e7) to head (bd3505e).
Report is 3 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #8212   +/-   ##
=======================================
  Coverage   95.51%   95.51%           
=======================================
  Files         404      404           
  Lines       11531    11531           
  Branches     3830     3829    -1     
=======================================
  Hits        11014    11014           
  Misses        517      517           
Flag Coverage Δ
unittests 95.51% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@EldarMuhamethanov EldarMuhamethanov marked this pull request as ready for review January 30, 2025 10:56
Copy link
Contributor

@andrey-medvedev-vk andrey-medvedev-vk left a comment

Choose a reason for hiding this comment

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

👏 👏 👏

SevereCloud
SevereCloud previously approved these changes Jan 30, 2025
SevereCloud
SevereCloud previously approved these changes Jan 31, 2025
Copy link
Contributor

@andrey-medvedev-vk andrey-medvedev-vk left a comment

Choose a reason for hiding this comment

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

🔥

Copy link
Contributor

@inomdzhon inomdzhon left a comment

Choose a reason for hiding this comment

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

👏 👍 🚀

@EldarMuhamethanov EldarMuhamethanov merged commit 5e4a670 into master Feb 3, 2025
29 checks passed
@EldarMuhamethanov EldarMuhamethanov deleted the e.muhamethanov/flex-gap-support branch February 3, 2025 13:16
@EldarMuhamethanov EldarMuhamethanov added this to the v7.2.0 milestone Feb 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants