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.

Sign up for GitHub

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

Jump to bottom

plasma-new-hope: fix id generation for Modal, Popup #1001

Merged
merged 2 commits into from
Jan 25, 2024
Merged

plasma-new-hope: fix id generation for Modal, Popup #1001

merged 2 commits into from
Jan 25, 2024

Conversation

kayman233
Copy link
Contributor

@kayman233 kayman233 commented Jan 24, 2024 •
edited by Salute-Eva
Loading

Modal

  • поправлена генерация id для Modal, Popup
  • добавлены тесты для Modal, Popup

What/why changed

Ломалось использование ModalBase без id из-за того, что неправильно пробрасывали. Теперь также используем safeUseId вместо useUniqId. Добавлены тесты для web, b2c для обоих компонент.

📦 Published PR as canary version: Canary Versions

✨ Test out this PR locally via:

npm install @salutejs/[email protected]
npm install @salutejs/[email protected]
npm install @salutejs/[email protected]
npm install @salutejs/[email protected]
# or 
yarn add @salutejs/[email protected]
yarn add @salutejs/[email protected]
yarn add @salutejs/[email protected]
yarn add @salutejs/[email protected]

@Salute-Eva
Copy link
Contributor

Theme Builder app deployed!

http://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-1001/

@Salute-Eva
Copy link
Contributor

Documentation preview deployed!

website: http://plasma.sberdevices.ru/pr/pr-1001/
b2c storybook: http://plasma.sberdevices.ru/pr/pr-1001/b2c-storybook/
web storybook: http://plasma.sberdevices.ru/pr/pr-1001/web-storybook/
new-hope storybook: http://plasma.sberdevices.ru/pr/pr-1001/new-hope-storybook/
asdk storybook: http://plasma.sberdevices.ru/pr/pr-1001/asdk-storybook/

@kayman233 kayman233 force-pushed the kayman233/fix-id-usage-modal.PLASMA-2364 branch from ce2627f to cd22432 Compare January 24, 2024 11:04
@Salute-Eva
Copy link
Contributor

Theme Builder app deployed!

http://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-1001/

@Salute-Eva
Copy link
Contributor

Documentation preview deployed!

website: http://plasma.sberdevices.ru/pr/pr-1001/
b2c storybook: http://plasma.sberdevices.ru/pr/pr-1001/b2c-storybook/
web storybook: http://plasma.sberdevices.ru/pr/pr-1001/web-storybook/
new-hope storybook: http://plasma.sberdevices.ru/pr/pr-1001/new-hope-storybook/
asdk storybook: http://plasma.sberdevices.ru/pr/pr-1001/asdk-storybook/

@kayman233 kayman233 marked this pull request as ready for review January 24, 2024 11:33
Yeti-or
Yeti-or approved these changes Jan 24, 2024
View reviewed changes
packages/plasma-new-hope/src/components/Modal/Modal/Modal.tsx
@@ -43,14 +43,14 @@ export const modalRoot = (Root: RootProps<HTMLDivElement, ModalProps>) =>

const innerRef = useForkRef<HTMLDivElement>(trapRef, outerRootRef);

const uniqId = useUniqId();
Copy link
Contributor

Choose a reason for hiding this comment

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

Может нам дропнуть useUniq из core ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

отдельной бы задачкой. А то plasma-ui весь на нём

Copy link
Contributor

Choose a reason for hiding this comment

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

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

@kayman233 kayman233 force-pushed the kayman233/fix-id-usage-modal.PLASMA-2364 branch from cd22432 to 671b5cb Compare January 25, 2024 09:59
@Salute-Eva
Copy link
Contributor

Theme Builder app deployed!

http://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-1001/

@Salute-Eva
Copy link
Contributor

Documentation preview deployed!

website: http://plasma.sberdevices.ru/pr/pr-1001/
b2c storybook: http://plasma.sberdevices.ru/pr/pr-1001/b2c-storybook/
web storybook: http://plasma.sberdevices.ru/pr/pr-1001/web-storybook/
new-hope storybook: http://plasma.sberdevices.ru/pr/pr-1001/new-hope-storybook/
asdk storybook: http://plasma.sberdevices.ru/pr/pr-1001/asdk-storybook/

@github-actions GitHub Actions
Copy link
Contributor

⚡ Component performance testing

Result: 🟢 OK

@kayman233 kayman233 added this pull request to the merge queue Jan 25, 2024
Merged via the queue into dev with commit 092a661 Jan 25, 2024
25 checks passed
@kayman233 kayman233 deleted the kayman233/fix-id-usage-modal.PLASMA-2364 branch January 25, 2024 10:40
@Salute-Eva
Copy link
Contributor

🚀 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] 🚀

This was referenced Jan 31, 2024
Closed
Merged
@Salute-Eva
Copy link
Contributor

🚀 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] 🚀

This was referenced Jul 24, 2024
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neretin-trike neretin-trike neretin-trike approved these changes

@Yeti-or Yeti-or Yeti-or approved these changes

@Yakutoc Yakutoc Awaiting requested review from Yakutoc Yakutoc is a code owner

Assignees
No one assigned
Labels
plasma-components released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kayman233 @Salute-Eva @Yeti-or @neretin-trike