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

FormTokenField: Issues with multiple identical strings #62533

Open
Luehrsen opened this issue Jun 13, 2024 · 2 comments
Open

FormTokenField: Issues with multiple identical strings #62533

Luehrsen opened this issue Jun 13, 2024 · 2 comments
Labels
[Package] Components /packages/components [Type] Bug An existing feature does not function as intended

Comments

@Luehrsen
Copy link
Contributor

Description

The FormTokenField component encounters issues when handling multiple tokens with identical string values. Specifically, when two or more items have the same name or title, the component fails to manage them correctly. This reliance on string matching without unique identifiers causes a React error and prevents selection of the duplicate items.

Expected Behavior:

  • The FormTokenField should allow selection and management of multiple tokens, even if they share the same name or title, without any errors.

Current Behavior:

  • When the FormTokenField encounters multiple items with the same name or title, a React/JavaScript error ‘Warning: Encountered two children with the same key’ is thrown.
  • The second (and any subsequent) item with the identical title becomes unselectable.

P.S.: I am aware that the FormTokenField is currently being rewritten. However, I believe it is important to report this issue regardless.

Step-by-step reproduction instructions

  1. Create a post tag called 'Identical Name'.

    • Navigate to the "Tags" section in the WordPress admin dashboard.
    • Add a new tag with the name 'Identical Name'.
  2. Create a second post tag called 'Identical Name2'.

    • In the same "Tags" section, add another tag with the name 'Identical Name2'.
    • Note: Creation of a tag with an existing name (e.g., 'Identical Name') is blocked by WordPress.
  3. Edit the second post tag to be 'Identical Name'.

    • Find the tag 'Identical Name2' in the list of tags.
    • Edit the tag and change its name to 'Identical Name'.
    • Note: The check on existing names is not active on editing tags.
  4. Create a new post.

    • Navigate to "Posts" in the WordPress admin dashboard.
    • Click "Add New" to create a new post.
  5. Edit post tags.

    • In the block editor, locate the tags input field.
  6. Type 'Identical Name' in the FormTokenField.

    • Begin typing 'Identical Name' in the tags input field.
    • Observe the behavior of the FormTokenField as it attempts to handle the duplicate tags.

Screenshots, screen recording, code snippet

No response

Environment info

  • WordPress 6.6-beta2-58392-src

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@mirka
Copy link
Member

mirka commented Jul 8, 2024

Thanks for the report!

It is true that somebody needs to be responsible for ensuring uniqueness, but ultimately I don't think it can/should be FormTokenField itself. The consumer knows best about how it wants to deal with potential duplicates, and even if we required unique identifiers, the consumer would still be responsible for ensuring identifier uniqueness.

The repro instructions you posted are in fact problematic at the application level and can be considered a bug though. Would it be fair to reclassify that as an app bug, rather than a problem with FormTokenField itself?

@Luehrsen
Copy link
Contributor Author

Luehrsen commented Jul 8, 2024

Thanks for the discussion and insights on this issue.

I would like to clarify a few points regarding the responsibility of not relying on uniqueness in FormTokenField:

  1. Issue with FormTokenField: This is indeed an issue with FormTokenField. The control should inherently handle potential duplicates by not relying on the uniqueness of the tokens. Instead, it should at least provide options for utilizing unique identifiers.

  2. Repro Instructions: The repro instructions were intended to illustrate that we cannot and should not assume that the tokens, which this control operates upon, will always be unique. It’s crucial to design the control to handle such scenarios gracefully.

  3. String Matching: Relying on string matching for uniqueness is generally a code smell. This approach can lead to various issues, especially when unique identifiers are available and can be utilized effectively to manage tokens.

  4. Edge Cases Handling: The presence of an edge case and a code comment in getTermIdByTermValue discussing non-unique tokens indicates that this is a known issue. Addressing it at the control level would provide a more comprehensive solution, ensuring that all edge cases are handled consistently.

  5. Performance Considerations: Implementing token management within the control can be optimized for performance. This centralized approach allows for more efficient handling of tokens, as opposed to having multiple consumers implement their own potentially less efficient solutions.

  6. Aligning with HTML Select Tag: By adding a unique identifier, we move FormTokenField more in line with the HTML select tag, which traditionally and by design supports separate keys and values. This alignment ensures a consistent developer experience and leverages established design patterns.

Side Note: After re-reading the implementation of getTermIdByTermValue, I see that the string is lowercased. So a value of EXAMPLE and exAMple would be considered equal, and the latter would not be selectable. There might be more underlying issues, especially in languages with extended character sets.

Given these points, it is evident that addressing the issue within FormTokenField is not only beneficial but necessary to ensure a robust, maintainable, and user-friendly control.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

3 participants