Skip to content

Commit 185bab1

Browse files
shrabantipaul-collateShrabanti Paul
authored andcommitted
Fixes 2931: Fix issue hitting enter on input is clearing pre-added tags (#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)
1 parent 0e4e6f2 commit 185bab1

3 files changed

Lines changed: 202 additions & 0 deletions

File tree

openmetadata-ui/src/main/resources/ui/src/components/SettingsSso/SSOConfigurationForm/SSOConfigurationForm.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ import {
5757
clearFieldError,
5858
createDOMClickHandler,
5959
createDOMFocusHandler,
60+
createFormKeyDownHandler,
6061
createFreshFormData,
6162
findChangedFields,
6263
getProviderDisplayName,
@@ -542,16 +543,19 @@ const SSOConfigurationFormRJSF = ({
542543
useEffect(() => {
543544
const handleDOMFocus = createDOMFocusHandler(setActiveField);
544545
const handleDOMClick = createDOMClickHandler(setActiveField);
546+
const handleKeyDown = createFormKeyDownHandler();
545547

546548
// Add event listeners when form is shown
547549
if (showForm) {
548550
document.addEventListener('focusin', handleDOMFocus);
549551
document.addEventListener('click', handleDOMClick);
552+
document.addEventListener('keydown', handleKeyDown, true);
550553
}
551554

552555
return () => {
553556
document.removeEventListener('focusin', handleDOMFocus);
554557
document.removeEventListener('click', handleDOMClick);
558+
document.removeEventListener('keydown', handleKeyDown, true);
555559
};
556560
}, [showForm]);
557561

openmetadata-ui/src/main/resources/ui/src/utils/SSOUtils.test.ts

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
clearFieldError,
2121
createDOMClickHandler,
2222
createDOMFocusHandler,
23+
createFormKeyDownHandler,
2324
createFreshFormData,
2425
extractFieldName,
2526
findChangedFields,
@@ -2797,4 +2798,173 @@ describe('SSOUtils', () => {
27972798
expect(hasFieldValidationErrors(error)).toBe(true);
27982799
});
27992800
});
2801+
2802+
describe('createFormKeyDownHandler', () => {
2803+
let handler: (e: KeyboardEvent) => void;
2804+
let mockEvent: Partial<KeyboardEvent>;
2805+
2806+
beforeEach(() => {
2807+
handler = createFormKeyDownHandler();
2808+
mockEvent = {
2809+
key: 'Enter',
2810+
preventDefault: jest.fn(),
2811+
stopPropagation: jest.fn(),
2812+
};
2813+
});
2814+
2815+
describe('should prevent default for regular input fields', () => {
2816+
it('should prevent default when Enter is pressed in INPUT element', () => {
2817+
const input = document.createElement('input');
2818+
Object.defineProperty(mockEvent, 'target', {
2819+
value: input,
2820+
writable: true,
2821+
});
2822+
2823+
handler(mockEvent as KeyboardEvent);
2824+
2825+
expect(mockEvent.preventDefault).toHaveBeenCalled();
2826+
expect(mockEvent.stopPropagation).toHaveBeenCalled();
2827+
});
2828+
2829+
it('should prevent default when Enter is pressed in Ant Design input field', () => {
2830+
const div = document.createElement('div');
2831+
div.className = 'ant-input';
2832+
Object.defineProperty(mockEvent, 'target', {
2833+
value: div,
2834+
writable: true,
2835+
});
2836+
2837+
handler(mockEvent as KeyboardEvent);
2838+
2839+
expect(mockEvent.preventDefault).toHaveBeenCalled();
2840+
expect(mockEvent.stopPropagation).toHaveBeenCalled();
2841+
});
2842+
});
2843+
2844+
describe('should NOT prevent default for special cases', () => {
2845+
it('should NOT prevent default when Enter is pressed in TEXTAREA', () => {
2846+
const textarea = document.createElement('textarea');
2847+
Object.defineProperty(mockEvent, 'target', {
2848+
value: textarea,
2849+
writable: true,
2850+
});
2851+
2852+
handler(mockEvent as KeyboardEvent);
2853+
2854+
expect(mockEvent.preventDefault).not.toHaveBeenCalled();
2855+
expect(mockEvent.stopPropagation).not.toHaveBeenCalled();
2856+
});
2857+
2858+
it('should NOT prevent default when Enter is pressed in Select tags component', () => {
2859+
const input = document.createElement('input');
2860+
const selector = document.createElement('div');
2861+
selector.className = 'ant-select-selector';
2862+
selector.appendChild(input);
2863+
Object.defineProperty(mockEvent, 'target', {
2864+
value: input,
2865+
writable: true,
2866+
});
2867+
2868+
handler(mockEvent as KeyboardEvent);
2869+
2870+
expect(mockEvent.preventDefault).not.toHaveBeenCalled();
2871+
expect(mockEvent.stopPropagation).not.toHaveBeenCalled();
2872+
});
2873+
2874+
it('should NOT prevent default for non-Enter keys', () => {
2875+
const input = document.createElement('input');
2876+
Object.defineProperty(mockEvent, 'target', {
2877+
value: input,
2878+
writable: true,
2879+
});
2880+
Object.defineProperty(mockEvent, 'key', {
2881+
value: 'Tab',
2882+
writable: true,
2883+
});
2884+
2885+
handler(mockEvent as KeyboardEvent);
2886+
2887+
expect(mockEvent.preventDefault).not.toHaveBeenCalled();
2888+
expect(mockEvent.stopPropagation).not.toHaveBeenCalled();
2889+
});
2890+
2891+
it('should NOT prevent default for non-input elements', () => {
2892+
const div = document.createElement('div');
2893+
Object.defineProperty(mockEvent, 'target', {
2894+
value: div,
2895+
writable: true,
2896+
});
2897+
2898+
handler(mockEvent as KeyboardEvent);
2899+
2900+
expect(mockEvent.preventDefault).not.toHaveBeenCalled();
2901+
expect(mockEvent.stopPropagation).not.toHaveBeenCalled();
2902+
});
2903+
2904+
it('should NOT prevent default for button elements', () => {
2905+
const button = document.createElement('button');
2906+
Object.defineProperty(mockEvent, 'target', {
2907+
value: button,
2908+
writable: true,
2909+
});
2910+
2911+
handler(mockEvent as KeyboardEvent);
2912+
2913+
expect(mockEvent.preventDefault).not.toHaveBeenCalled();
2914+
expect(mockEvent.stopPropagation).not.toHaveBeenCalled();
2915+
});
2916+
});
2917+
2918+
describe('edge cases', () => {
2919+
it('should handle null target gracefully', () => {
2920+
Object.defineProperty(mockEvent, 'target', {
2921+
value: null,
2922+
writable: true,
2923+
});
2924+
2925+
expect(() => handler(mockEvent as KeyboardEvent)).not.toThrow();
2926+
expect(mockEvent.preventDefault).not.toHaveBeenCalled();
2927+
});
2928+
2929+
it('should handle undefined target gracefully', () => {
2930+
Object.defineProperty(mockEvent, 'target', {
2931+
value: undefined,
2932+
writable: true,
2933+
});
2934+
2935+
expect(() => handler(mockEvent as KeyboardEvent)).not.toThrow();
2936+
expect(mockEvent.preventDefault).not.toHaveBeenCalled();
2937+
});
2938+
2939+
it('should prevent default for nested input inside ant-input div', () => {
2940+
const input = document.createElement('input');
2941+
const antInputDiv = document.createElement('div');
2942+
antInputDiv.className = 'ant-input';
2943+
antInputDiv.appendChild(input);
2944+
Object.defineProperty(mockEvent, 'target', {
2945+
value: antInputDiv,
2946+
writable: true,
2947+
});
2948+
2949+
handler(mockEvent as KeyboardEvent);
2950+
2951+
expect(mockEvent.preventDefault).toHaveBeenCalled();
2952+
expect(mockEvent.stopPropagation).toHaveBeenCalled();
2953+
});
2954+
2955+
it('should prevent default for input with multiple classes including ant-input', () => {
2956+
const div = document.createElement('div');
2957+
div.className = 'custom-class ant-input another-class';
2958+
Object.defineProperty(mockEvent, 'target', {
2959+
value: div,
2960+
writable: true,
2961+
});
2962+
2963+
handler(mockEvent as KeyboardEvent);
2964+
2965+
expect(mockEvent.preventDefault).toHaveBeenCalled();
2966+
expect(mockEvent.stopPropagation).toHaveBeenCalled();
2967+
});
2968+
});
2969+
});
28002970
});

openmetadata-ui/src/main/resources/ui/src/utils/SSOUtils.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1045,3 +1045,31 @@ export const hasFieldValidationErrors = (
10451045
'errors' in axiosError.response.data
10461046
);
10471047
};
1048+
1049+
/**
1050+
* Creates a keydown event handler that prevents form submission when Enter is pressed in input fields
1051+
* This prevents array field tags from being removed when Enter is pressed in other fields
1052+
* @returns Event handler function for keydown events
1053+
*/
1054+
export const createFormKeyDownHandler = () => {
1055+
return (e: KeyboardEvent) => {
1056+
if (
1057+
e.key === 'Enter' &&
1058+
e.target &&
1059+
(e.target as HTMLElement).tagName !== 'TEXTAREA'
1060+
) {
1061+
const target = e.target as HTMLElement;
1062+
1063+
if (
1064+
target.tagName === 'INPUT' ||
1065+
target.classList.contains('ant-input')
1066+
) {
1067+
const isInSelectTags = target.closest('.ant-select-selector');
1068+
if (!isInSelectTags) {
1069+
e.preventDefault();
1070+
e.stopPropagation();
1071+
}
1072+
}
1073+
}
1074+
};
1075+
};

0 commit comments

Comments
 (0)