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

Fix ui switcher element not cleaned up on input destroy #37512

Open
wants to merge 4 commits into
base: 2.4-develop
Choose a base branch
from
Open

Fix ui switcher element not cleaned up on input destroy #37512

wants to merge 4 commits into from

Conversation

smartexcan
Copy link

@smartexcan smartexcan commented May 17, 2023 •
edited
Loading

Description (*)

The ui switcher element is created by the ui form field element if it is enabled, and is added to the uiRegistry when it is initialized.
If the form field is then destroyed (either manually or from a destroyInserted of an insert-form) the switcher is not removed from the uiRegistry.

This causes an issue if that form field is created again, as the previous switcher still exists and a new instance is not created, causing it to not be linked to the new form field.

Related Pull Requests

Fixed Issues (if relevant)

fixes #39553

Manual testing scenarios (*)

  1. Create two ui component forms.
  2. In the first form, create a button component, and a modal component with an insert-form component.
  3. Set the button's actions to destroyInserted on the insert-form, openModal on the modal, and render on the insert-form.
  4. Set the insert-form to render second form, and set autoRender to false.
  5. Add a checkbox field (default unchecked) and an input field to the second form.
  6. Set up switcherConfig for the checkbox field so that it enables/disables the input field when the checkbox is toggled.
  7. Open the first form and click the button to open the modal with the second form.
  8. Toggle the checkbox to enable the input field and enter a value, then close the modal.
  9. Open the modal again and toggle the checkbox. The input field should be enabled/disabled as expected.

Questions or comments

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
  • All automated tests passed successfully (all builds are green)

@m2-assistant
Copy link

m2-assistant bot commented May 17, 2023

Hi @smartexcan. Thank you for your contribution!
Here are some useful tips on how you can test your changes using Magento test environment.

Add the comment under your pull request to deploy test or vanilla Magento instance:
  • @magento give me test instance - deploy test instance based on PR changes
  • @magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

  • @magento run all tests - run or re-run all required tests against the PR changes
  • @magento run <test-build(s)> - run or re-run specific test build(s)
    For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names.

Allowed build names are:
  1. Database Compare
  2. Functional Tests CE
  3. Functional Tests EE
  4. Functional Tests B2B
  5. Integration Tests
  6. Magento Health Index
  7. Sample Data Tests CE
  8. Sample Data Tests EE
  9. Sample Data Tests B2B
  10. Static Tests
  11. Unit Tests
  12. WebAPI Tests
  13. Semantic Version Checker

You can find more information about the builds here
ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.

For more details, review the Code Contributions documentation.
Join Magento Community Engineering Slack and ask your questions in #github channel.

@smartexcan
Copy link
Author

@magento run all tests

@magento-automated-testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

@engcom-Bravo engcom-Bravo added the Priority: P2 A defect with this priority could have functionality issues which are not to expectations. label May 18, 2023
@smartexcan
Copy link
Author

@magento run all tests

@magento-automated-testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

@smartexcan
Copy link
Author

@magento run all tests

@magento-automated-testing Magento Automated Testing
Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

@smartexcan
Copy link
Author

@magento run all tests

@smartexcan smartexcan mentioned this pull request Jan 20, 2025
Open
5 tasks
@xtremevision
Copy link

This should be merged asap. It is a critical fix to broken functionality as reported by me in #39553

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: review
Projects
Pull Requests Dashboard
Status: Review in Progress
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

switcherConfig stops working in admin customer address edit modal
4 participants
@smartexcan @xtremevision @engcom-Bravo @magento-ct-gh-projects-svc