Skip to content

Fixes 2931: Fix issue hitting enter on input is clearing pre-added tags#26039

Merged
chirag-madlani merged 4 commits intomainfrom
bugfix/sso-form-enter-issue
Feb 23, 2026
Merged

Fixes 2931: Fix issue hitting enter on input is clearing pre-added tags#26039
chirag-madlani merged 4 commits intomainfrom
bugfix/sso-form-enter-issue

Conversation

@shrabantipaul-collate
Copy link
Copy Markdown
Contributor

@shrabantipaul-collate shrabantipaul-collate commented Feb 23, 2026

Describe your changes:

Fixes https://github.com/open-metadata/openmetadata-collate/issues/2931

I worked on ... because ...

Problem

When pressing Enter in any input field on the SSO Configuration Form, pre-added tags in Select (tag mode) fields were being unexpectedly cleared.

Root Cause

Enter keydown events on inputs bubbled up and triggered Ant Design's internal tag-removal logic on nearby Select components.

Solution

Added createFormKeyDownHandler utility in SSOUtils.ts that attaches a keydown listener to the form, intercepting Enter on input fields via preventDefault + stopPropagation. The handler explicitly skips:

  • TEXTAREA elements (multi-line input should work normally)
  • Inputs inside .ant-select-selector (so Select tag fields still work)
Screen.Recording.2026-02-23.at.1.04.50.PM.mov

Type of change:

  • Bug fix
  • Improvement
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

Summary by Gitar

  • Fixed tag clearing bug:
    • Added createFormKeyDownHandler utility to prevent Enter keydown events from bubbling up and triggering Ant Design Select tag removal
    • Handler excludes textarea and Select tag selector inputs to preserve expected behavior
  • Test coverage:
    • Added 170 lines of comprehensive tests covering normal cases, exceptions (textarea, Select selectors), and edge cases (null targets, nested structures)

This will update automatically on new commits.

@github-actions
Copy link
Copy Markdown
Contributor

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Feb 23, 2026

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 65%
65.74% (56417/85817) 45.18% (29545/65397) 47.99% (8908/18563)

Comment thread openmetadata-ui/src/main/resources/ui/src/utils/SSOUtils.ts Outdated
Comment thread openmetadata-ui/src/main/resources/ui/src/utils/SSOUtils.ts Outdated
@gitar-bot
Copy link
Copy Markdown

gitar-bot Bot commented Feb 23, 2026

🔍 CI failure analysis for 70db886: Multiple Playwright CI shards (1/6 and 4/6) failed with test failures in AuditLogs, LineageSettings, and CustomProperties areas. All failures are unrelated to this PR's SSO form changes.

Issue

Multiple Playwright CI shards failed with test failures across different areas.

Root Cause

The failures are unrelated to this PR's changes. This PR modifies only SSO configuration form files:

  • SSOConfigurationForm.tsx
  • SSOUtils.ts
  • SSOUtils.test.ts

All test failures occur in completely different areas with no connection to SSO forms.

Details

Shard 1/6 Failures

Failed Tests (2)

  • AuditLogs Export (2 tests): Date picker strict mode violation - .ant-picker-cell-today resolved to 2 elements

Flaky (1)

  • AuditLogs filters: filter-chip-entityType not visible

Shard 4/6 Failures

Failed (1)

  • LineageSettings › Verify global lineage config

Flaky (11)

  • CustomProperties DateTime: After reload, property shows "Not set" instead of persisted value
  • CustomProperties Date/Integer/String/Duration: Various persistence and timing issues
  • ExploreDiscovery: Deleted assets display problems

Common Patterns

  • Custom property persistence failures - Values not saving across page reloads
  • Date picker strict mode violations - Multiple picker instances
  • Element visibility timing - UI elements not appearing reliably
  • Test flakiness - Operations timing out, stale data

These are pre-existing test infrastructure issues. The SSO keyboard event handling and Jest test fixes in this PR have no connection to AuditLogs, LineageSettings, or CustomProperties functionality.

Code Review 👍 Approved with suggestions 2 resolved / 2 findings

Clean, well-tested fix for Enter key event propagation clearing Select tags. The implementation follows existing patterns in the codebase with proper cleanup. No new issues beyond the two already-posted findings.

✅ 2 resolved
Bug: Dead code: ant-select-search__field is an Ant Design v3 class

📄 openmetadata-ui/src/main/resources/ui/src/utils/SSOUtils.ts:1106
The handler checks for target.classList.contains('ant-select-search__field') on line 1106, but this is an Ant Design v3 class name. This project uses Ant Design v4.24.16, where the equivalent class is ant-select-selection-search-input — which is used in 6 other files in this codebase, including the SSO form's own styles (sso-configuration-form-array-field-template.less).

This condition will never match at runtime, making it dead code. The fix still works correctly because Select search inputs render as <input> elements caught by target.tagName === 'INPUT', but the dead condition is misleading and could cause confusion during future maintenance.

Consider replacing ant-select-search__field with ant-select-selection-search-input to match the actual Ant Design v4 class used throughout the codebase. The corresponding test case (line 2843-2855) should also be updated.

Quality: Redundant null check: target already accessed before guard

📄 openmetadata-ui/src/main/resources/ui/src/utils/SSOUtils.ts:1094-1102
On line 1096, (e.target as HTMLElement)?.tagName is accessed with optional chaining. If e.target is null/undefined, this evaluates to undefined !== 'TEXTAREA' which is true, so execution enters the block. Then on line 1098, target is assigned as e.target as HTMLElement, and line 1100 checks if (!target).

While this works correctly at runtime (the null check on line 1100 does catch null targets), the code flow is confusing — it accesses .tagName on the target before checking if the target exists. Consider restructuring to check for target existence first:

return (e: KeyboardEvent) => {
  const target = e.target as HTMLElement;
  if (!target || e.key !== 'Enter' || target.tagName === 'TEXTAREA') {
    return;
  }
  // ... rest of logic
};

This is clearer about the order of operations and avoids the appearance of a potential null dereference.

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqubecloud
Copy link
Copy Markdown

@chirag-madlani chirag-madlani merged commit 2086d37 into main Feb 23, 2026
20 of 22 checks passed
@chirag-madlani chirag-madlani deleted the bugfix/sso-form-enter-issue branch February 23, 2026 15:04
github-actions Bot pushed a commit that referenced this pull request Feb 23, 2026
…gs (#26039)

* Fix issue entering on input is clearing pre-added tags

* address gitar bot review comments

* fix failing jest tests

---------

Co-authored-by: Shrabanti Paul <shrabantipaul@Shrabantis-MacBook-Pro.local>
(cherry picked from commit 2086d37)
@github-actions
Copy link
Copy Markdown
Contributor

Changes have been cherry-picked to the 1.11.11 branch.

@shrabantipaul-collate shrabantipaul-collate restored the bugfix/sso-form-enter-issue branch February 25, 2026 05:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs To release Will cherry-pick this PR into the release branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants