fix: review comments

Author: Elessar1802

src/Common/Hooks/UseRegisterShortcut/UseRegisterShortcutProvider.tsx

@@ -26,8 +26,10 @@ const DEFAULT_TIMEOUT = 300
 const UseRegisterShortcutProvider = ({
     ignoreTags,
     preventDefault = false,
+    stopPropagation = false,
     shortcutTimeout,
     children,
+    eventListenerTargetRef,
 }: UseRegisterShortcutProviderType) => {
     const disableShortcutsRef = useRef<boolean>(false)
     const shortcutsRef = useRef<Record<string, ShortcutType>>({})
@@ -120,6 +122,10 @@ const UseRegisterShortcutProvider = ({
             event.preventDefault()
         }
 
+        if (stopPropagation) {
+            event.stopPropagation()
+        }
+
         if (
             ignoredTags.map((tag) => tag.toUpperCase()).indexOf((event.target as HTMLElement).tagName.toUpperCase()) >
                 -1 ||
@@ -156,14 +162,16 @@ const UseRegisterShortcutProvider = ({
     }, [])
 
     useEffect(() => {
-        window.addEventListener('keydown', handleKeydownEvent)
-        window.addEventListener('keyup', handleKeyupEvent)
-        window.addEventListener('blur', handleBlur)
+        const target = eventListenerTargetRef?.current ?? window
+
+        target.addEventListener('keydown', handleKeydownEvent)
+        target.addEventListener('keyup', handleKeyupEvent)
+        target.addEventListener('blur', handleBlur)
 
         return () => {
-            window.removeEventListener('keydown', handleKeydownEvent)
-            window.removeEventListener('keyup', handleKeyupEvent)
-            window.removeEventListener('blur', handleBlur)
+            target.removeEventListener('keydown', handleKeydownEvent)
+            target.removeEventListener('keyup', handleKeyupEvent)
+            target.removeEventListener('blur', handleBlur)
 
             if (keyDownTimeoutRef.current > -1) {
                 clearTimeout(keyDownTimeoutRef.current)

src/Common/Hooks/UseRegisterShortcut/types.ts

@@ -93,4 +93,13 @@ export interface UseRegisterShortcutProviderType {
      * If true, call preventDefault on the event
      */
     preventDefault?: boolean
+    /**
+     * If true, call stopPropagation on the event
+     */
+    stopPropagation?: boolean
+    /**
+     * The target to which the event listener is attached
+     * If not provided, the event listener will be attached to window
+     */
+    eventListenerTargetRef?: React.RefObject<HTMLElement>
 }

src/Common/Hooks/useUrlFilters/types.ts

@@ -16,7 +16,7 @@
 
 import { SortingOrder } from '../../Constants'
 
-export interface UseUrlFiltersProps<T, K> {
+export interface UseUrlFiltersProps<T, K extends {}> {
     /**
      * The key on which the sorting should be applied
      */
@@ -37,9 +37,9 @@ export interface UseUrlFiltersProps<T, K> {
     redirectionMethod?: 'replace' | 'push'
 }
 
-export type UpdateSearchParamsOptionsType<T, K = unknown> = Partial<Pick<UseUrlFiltersProps<T, K>, 'redirectionMethod'>>
+export type UpdateSearchParamsOptionsType<T, K = {}> = Partial<Pick<UseUrlFiltersProps<T, K>, 'redirectionMethod'>>
 
-export type UseUrlFiltersReturnType<T, K = unknown> = K & {
+export type UseUrlFiltersReturnType<T, K = {}> = K & {
     /**
      * Currently applied page size
      */

src/Common/Hooks/useUrlFilters/useUrlFilters.ts

@@ -20,7 +20,7 @@ import { getUrlWithSearchParams } from '@Common/Helper'
 import { DEFAULT_BASE_PAGE_SIZE, EXCLUDED_FALSY_VALUES, SortingOrder } from '../../Constants'
 import { DEFAULT_PAGE_NUMBER, URL_FILTER_KEYS } from './constants'
 import { UpdateSearchParamsOptionsType, UseUrlFiltersProps, UseUrlFiltersReturnType } from './types'
-import { setItemInLocalStorageIfKeyExists } from './utils'
+import { areAnyAdditionalFiltersApplied, setItemInLocalStorageIfKeyExists } from './utils'
 
 const { PAGE_SIZE, PAGE_NUMBER, SEARCH_KEY, SORT_BY, SORT_ORDER } = URL_FILTER_KEYS
 
@@ -41,7 +41,8 @@ const { PAGE_SIZE, PAGE_NUMBER, SEARCH_KEY, SORT_BY, SORT_ORDER } = URL_FILTER_K
  * ```
  *
  */
-const useUrlFilters = <T = string, K = unknown>({
+
+const useUrlFilters = <T = string, K = {}>({
     initialSortKey,
     parseSearchParams,
     localStorageKey,
@@ -220,7 +221,7 @@ const useUrlFilters = <T = string, K = unknown>({
         clearFilters,
         ...parsedParams,
         updateSearchParams,
-        isFilterApplied: !!Object.keys(parsedParams).length || !!searchKey,
+        isFilterApplied: !!searchKey || areAnyAdditionalFiltersApplied(parsedParams),
     }
 }
 

src/Common/Hooks/useUrlFilters/utils.tsx

@@ -14,8 +14,32 @@
  * limitations under the License.
  */
 
+import { isNullOrUndefined } from '@Shared/Helpers'
+
 export const setItemInLocalStorageIfKeyExists = (localStorageKey: string, value: string) => {
     if (localStorageKey) {
         localStorage.setItem(localStorageKey, value)
     }
 }
+
+export const areAnyAdditionalFiltersApplied = (parsedParams: Record<string | number, any>) => {
+    if (!parsedParams || !Object.keys(parsedParams).length) {
+        return false
+    }
+
+    return Object.keys(parsedParams).some((key) => {
+        if (isNullOrUndefined(parsedParams[key])) {
+            return false
+        }
+
+        if (Array.isArray(parsedParams[key])) {
+            return parsedParams[key].length > 0
+        }
+
+        if (typeof parsedParams[key] === 'string') {
+            return parsedParams[key].length > 0
+        }
+
+        return true
+    })
+}

src/Common/Tooltip/ShortcutKeyComboTooltipContent.tsx

@@ -23,9 +23,10 @@ const ShortcutKeyComboTooltipContent = ({ text, combo }: TooltipProps['shortcutK
         {!!combo?.length && (
             <div className="flexbox dc__gap-4 dc__align-items-center flex-wrap">
                 {combo.map((key) => (
-                    <span key={key} className="shortcut-keys__chip dc__capitalize lh-16 fs-11 fw-5 flex">
+                    // TODO: check styling for this since span was replaced by kbd
+                    <kbd key={key} className="shortcut-keys__chip dc__capitalize lh-16 fs-11 fw-5 flex">
                         {KEYBOARD_KEYS_MAP[key]}
-                    </span>
+                    </kbd>
                 ))}
             </div>
         )}

src/Shared/Components/Table/BulkSelectionActionWidget.tsx

@@ -9,6 +9,7 @@ import { useRegisterShortcut } from '@Common/Hooks'
 import { useEffect } from 'react'
 import { BulkSelectionActionWidgetProps } from './types'
 import { Button, ButtonComponentType, ButtonStyleType, ButtonVariantType } from '../Button'
+import { DRAG_SELECTOR_IDENTIFIER } from './constants'
 
 const BulkSelectionActionWidget = ({
     count,
@@ -28,17 +29,17 @@ const BulkSelectionActionWidget = ({
 
     return (
         <DraggableWrapper
-            dragSelector=".drag-selector"
+            dragSelector={`.${DRAG_SELECTOR_IDENTIFIER}`}
             positionVariant={DraggablePositionVariant.PARENT_BOTTOM_CENTER}
             zIndex="calc(var(--modal-index) - 1)"
             parentRef={parentRef}
         >
-            <div className="dc__separated-flexbox dc__separated-flexbox--gap-8 pt-12 pb-12 pr-12 pl-12 bulk-selection-widget br-8">
+            <div className="dc__separated-flexbox dc__separated-flexbox--gap-8 p-12 bulk-selection-widget br-8">
                 <div className="flexbox dc__gap-8">
-                    <DraggableButton dragClassName="drag-selector" />
+                    <DraggableButton dragClassName={DRAG_SELECTOR_IDENTIFIER} />
 
                     <div className="fs-13 lh-20 fw-6 flex dc__gap-12">
-                        <span className="flex dc__gap-2 bcb-5 cn-0 br-4 pr-6 pl-6">{count}</span>
+                        <span className="flex dc__gap-2 bcb-5 text__white br-4 px-6">{count}</span>
                         <span className="cn-9">Selected</span>
                     </div>
                 </div>
@@ -54,7 +55,7 @@ const BulkSelectionActionWidget = ({
                     ariaLabel="Clear selection(s)"
                     size={ComponentSizeType.small}
                     onClick={handleClearBulkSelection}
-                    showAriaLabelInTippy
+                    showAriaLabelInTippy={false}
                 />
             </div>
         </DraggableWrapper>