Skip to content

Conversation

SkyeYoung
Copy link
Member

@SkyeYoung SkyeYoung commented Aug 15, 2025

Please answer these questions before submitting a pull request, or your PR will get closed.

Why submit this pull request?

  • Bugfix
  • New feature provided
  • Improve performance
  • Backport patches

What changes will this PR take into?

The original code only updated data when it was first mounted, and the timing of resetting form data was uncertain, which often led to rendering failures.

// The method of applying API data in the detail/edit page
useEffect(() => {
  if (sslData && !isLoading) {
    form.reset(produceToSSLForm(sslData));
  }
}, [sslData, form, isLoading]);

Now, it's directly changed to useMemo, and the original useListState used for data maintenance has been removed, simplifying data maintenance and ensuring that data can be updated correctly.

For the testing part, it has not yet been decided whether to add unit tests, so I've added ssls check labels tests separately to clarify the testing objectives. This can also help fill a part of #3087 to a certain extent.

Related issues

fix/resolve #3172

Checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a bug in the FormItem Labels component where labels were not updating properly in forms. The main change replaces the complex state management using useListState and useMount with a simpler useMemo approach that directly derives values from form data.

  • Simplifies label state management in FormItem component by removing useListState hook
  • Updates import syntax to use named imports for clsx
  • Adds comprehensive test coverage for SSL label functionality

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/components/form/Labels.tsx Replaces useListState/useMount with useMemo for better label data synchronization
src/components/form/Editor.tsx Updates clsx import to use named import syntax
src/components/form-slice/FormSection/index.tsx Updates clsx import to use named import syntax
src/apis/ssls.ts Adds utility function for deleting all SSL records
e2e/utils/ui/ssls.ts New utility functions for SSL form testing
e2e/utils/ui/labels.ts New utility functions for label testing
e2e/tests/ssls.check-labels.spec.ts New e2e test covering label functionality
e2e/pom/ssls.ts New page object model for SSL testing

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@SkyeYoung SkyeYoung merged commit f5793d2 into apache:master Aug 19, 2025
5 checks passed
@SkyeYoung SkyeYoung deleted the young/fix/FormItem/Labels branch August 19, 2025 03:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

labels are missing after SSL creation
3 participants