-
Notifications
You must be signed in to change notification settings - Fork 20
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
feat(plasma-b2c, plasma-web): Modal refinement #629
Conversation
Theme Builder app deployed! http://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-629/ |
Documentation preview deployed! website: http://plasma.sberdevices.ru/pr/pr-629/ |
Component performance testing Result: 🟢 Success Check out report in job artifacts! |
0f3659c
to
347ca6b
Compare
Theme Builder app deployed! http://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-629/ |
Documentation preview deployed! website: http://plasma.sberdevices.ru/pr/pr-629/ |
Component performance testing Result: 🟢 Success Check out report in job artifacts! |
347ca6b
to
b535015
Compare
Theme Builder app deployed! http://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-629/ |
Documentation preview deployed! website: http://plasma.sberdevices.ru/pr/pr-629/ |
Component performance testing Result: 🟢 Success Check out report in job artifacts! |
b535015
to
951f5c1
Compare
Theme Builder app deployed! http://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-629/ |
Documentation preview deployed! website: http://plasma.sberdevices.ru/pr/pr-629/ |
Component performance testing Result: 🟢 Success Check out report in job artifacts! |
951f5c1
to
8049ce4
Compare
Theme Builder app deployed! http://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-629/ |
Documentation preview deployed! website: http://plasma.sberdevices.ru/pr/pr-629/ |
Component performance testing Result: 🟢 Success Check out report in job artifacts! |
focusLaterRef?: React_2.RefObject<HTMLElement>; | ||
initialFocusRef?: React_2.RefObject<HTMLElement>; | ||
isCloseOnEsc?: boolean; | ||
isCloseOnOverlayClick?: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
если честно мне не нравится писать через is
почему не просто
closeOnEsc?
у нас много мест где написано через is ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ну кстати да, в данном случае наверное без is лучше
@@ -906,7 +907,13 @@ export const Modal: FC<ModalProps>; | |||
|
|||
// @public (undocumented) | |||
export interface ModalProps extends ModalViewProps { | |||
focusLaterRef?: React_2.RefObject<HTMLElement>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
focusLater тоже звучит странно
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ну у меня в голове были еще focusAfter или focusAfterClose
@@ -923,6 +930,7 @@ export interface ModalViewProps extends React_2.HTMLAttributes<HTMLDivElement> { | |||
children?: React_2.ReactNode; | |||
closeButtonAriaLabel?: string; | |||
onClose?: () => void; | |||
showCloseIcon?: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
тут же унас не isShowCloseIcon
и почему icon ? именно в иконке дело ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
сделал showCloseButton, так кажется получше
@@ -25,8 +25,10 @@ export class FocusManager { | |||
}; | |||
|
|||
// добавление на фокус после анмаунта | |||
public markForFocusLater = () => { | |||
this.focusLaterElements.push(document.activeElement as HTMLElement); | |||
public markForFocusLater = (focusLaterNode?: React.RefObject<HTMLElement>) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
focusLater это уже в API focus-trap ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
да, ну логику focus-trap расширили
8049ce4
to
5c3a7e1
Compare
Theme Builder app deployed! http://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-629/ |
Documentation preview deployed! website: http://plasma.sberdevices.ru/pr/pr-629/ |
Component performance testing Result: 🟢 Success Check out report in job artifacts! |
5c3a7e1
to
1677f30
Compare
Theme Builder app deployed! http://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-629/ |
1677f30
to
6d52dd0
Compare
Theme Builder app deployed! http://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-629/ |
Documentation preview deployed! website: http://plasma.sberdevices.ru/pr/pr-629/ |
Component performance testing Result: 🟢 Success Check out report in job artifacts! |
🚀 This PR is included in version: @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected] 🚀 |
1 similar comment
🚀 This PR is included in version: @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected] 🚀 |
Добавлены новые свойства для компонента
Modal
:сloseOnEsc?: boolean
- Закрывать модальное окно при нажатии на ESC(по умолчанию true).сloseOnOverlayClick?: boolean
- Закрывать модальное окно при нажатии вне области модального окна(по умолчанию true).onEscKeyDown?: (event: KeyboardEvent) => void
- Обработчик клика при нажатии на ESC(если не передан, то при нажатии используется onClose).onOverlayClick?: (event: React.MouseEvent<HTMLDivElement>) => void
- Обработчик клика при нажатии вне области модального окна(если не передан, то при нажатии используется onClose).initialFocusRef?: React.RefObject<HTMLElement>
- Первый элемент для фокуса внутри модального окна.focusLaterRef?: React.RefObject<HTMLElement>
- Элемент для фокуса после закрытия модального окна(по умолчанию фокус на последнем перед открытием активном элементе).showCloseButton?: boolean
- Отображать кнопку "закрыть"(по умолчанию true).🐤 Download canary assets:
📦 Published PR as canary version:
Canary Versions
✨ Test out this PR locally via: