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

fix(base-modal): Скролл вверх модалки #224

Merged
merged 5 commits into from
Sep 13, 2022

Conversation

EGNKupava
Copy link
Contributor

@EGNKupava EGNKupava commented Aug 26, 2022

Опишите проблему

При открытии нескольких модалок. Если вызывающий следующую модалку контрол не мог принять фокус обратно, скролл проскакивал к первому фокусироемому элементу модалки. (В нашем случае обычно это крестик)

tabIndex для обертки позволяет фокусироваться на ней, при этом скролл сохраняет положение

При этом focus-lock рекомендуют управлять фокус локом самостоятельно, чтобы не было конфликтов при открытии нескольких модалок. Возможно стоит описать в доках, что в избежании багов фокус лок необходимо оставлять включенным только на верхней модалке

@EGNKupava
Copy link
Contributor Author

EGNKupava commented Aug 26, 2022

@core-ds-bot
Copy link
Collaborator

Собрана новая демка.

@core-ds-bot
Copy link
Collaborator

Собрана новая демка.

@coveralls
Copy link

coveralls commented Aug 26, 2022

Pull Request Test Coverage Report for Build 3039600722

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.009%) to 81.833%

Totals Coverage Status
Change from base Build 3038708464: 0.009%
Covered Lines: 5947
Relevant Lines: 6685

💛 - Coveralls

@core-ds-bot
Copy link
Collaborator

Собрана новая демка.

@reme3d2y
Copy link
Contributor

А оно точно работает? У нас на модалку накидывается zIndex из Stack, и он и так был 100, а не -1

image

@reme3d2y
Copy link
Contributor

Можешь прикрепить код, чтобы можно было увидеть все проблемы?

@EGNKupava EGNKupava closed this Aug 29, 2022
@EGNKupava EGNKupava reopened this Aug 29, 2022
@EGNKupava
Copy link
Contributor Author

EGNKupava commented Aug 29, 2022

Можешь прикрепить код, чтобы можно было увидеть все проблемы?

Есть песочницы было / стало. И там z-index. А нам нужен tab-index. Я думаю может пропс сделать focusableWrapper какой-нить или tabIndex И по дефолту оставить -1

@reme3d2y
Copy link
Contributor

Можешь прикрепить код, чтобы можно было увидеть все проблемы?

Есть песочницы было / стало. И там z-index. А нам нужен tab-index. Я думаю может пропс сделать focusableWrapper какой-нить или tabIndex И по дефолту оставить -1

сорри, перепутал, не проснулся еще :(

@core-ds-bot
Copy link
Collaborator

Собрана новая демка.

@core-ds-bot
Copy link
Collaborator

Собрана новая демка.

@SiebenSieben SiebenSieben merged commit 2e3fdd2 into master Sep 13, 2022
@SiebenSieben SiebenSieben deleted the fix/modal-focus branch September 13, 2022 07:18
core-ds-bot pushed a commit that referenced this pull request Sep 13, 2022
# [31.3.0](v31.2.0...v31.3.0) (2022-09-13)

### Bug Fixes

* **base-modal:** fixed scroll to top issues ([#224](#224)) ([2e3fdd2](2e3fdd2))
* **base-modal:** fixed ssr error in element matches polyfill ([#257](#257)) ([a6e05b1](a6e05b1))
* **file-upload-item:** fixed error display by default ([#252](#252)) ([874a669](874a669))

### Features

* **code-input:** update mobile version ([#230](#230)) ([bf66e85](bf66e85))
* **confirmation-v1:** rename package confirmation-v-1 ([#258](#258)) ([195f7a0](195f7a0))
* **confirmation-v1:** return old confirmation ([#241](#241)) ([a8779ed](a8779ed))
* **form-control, themes:** change colors in intranet theme ([#236](#236)) ([eae8b7d](eae8b7d))
* **plate:** added custom variable for box-shadow ([#256](#256)) ([4d75e4e](4d75e4e))
* **tabs:** update click theme  ([#225](#225)) ([fe12ef6](fe12ef6))
@core-ds-bot
Copy link
Collaborator

🎉 This PR is included in version 31.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@reme3d2y reme3d2y linked an issue Sep 20, 2022 that may be closed by this pull request
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.

Modal: disableRestoreFocus
7 participants