From 2cd3fe0dce6fdde6a3f61e596573669aee4bfcf7 Mon Sep 17 00:00:00 2001
From: lyzno1 <92089059+lyzno1@users.noreply.github.com>
Date: Tue, 5 Aug 2025 19:36:07 +0800
Subject: [PATCH] fix: Multiple UI component improvements and code quality
 enhancements (#23446)

---
 .../svg-attribute-error-reproduction.spec.tsx | 156 ++++++++++++++++++
 .../overview/tracing/config-button.tsx        |  27 +--
 .../[appId]/overview/tracing/panel.tsx        | 126 +++++++-------
 web/app/components/apps/footer.tsx            |   6 +-
 web/app/components/base/icons/utils.ts        |  17 ++
 .../components/base/tag-management/filter.tsx |   5 +-
 6 files changed, 249 insertions(+), 88 deletions(-)
 create mode 100644 web/app/(commonLayout)/app/(appDetailLayout)/[appId]/overview/tracing/__tests__/svg-attribute-error-reproduction.spec.tsx

diff --git a/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/overview/tracing/__tests__/svg-attribute-error-reproduction.spec.tsx b/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/overview/tracing/__tests__/svg-attribute-error-reproduction.spec.tsx
new file mode 100644
index 0000000000..a3281be8eb
--- /dev/null
+++ b/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/overview/tracing/__tests__/svg-attribute-error-reproduction.spec.tsx
@@ -0,0 +1,156 @@
+import React from 'react'
+import { render } from '@testing-library/react'
+import '@testing-library/jest-dom'
+import { OpikIconBig } from '@/app/components/base/icons/src/public/tracing'
+
+// Mock dependencies to isolate the SVG rendering issue
+jest.mock('react-i18next', () => ({
+  useTranslation: () => ({
+    t: (key: string) => key,
+  }),
+}))
+
+describe('SVG Attribute Error Reproduction', () => {
+  // Capture console errors
+  const originalError = console.error
+  let errorMessages: string[] = []
+
+  beforeEach(() => {
+    errorMessages = []
+    console.error = jest.fn((message) => {
+      errorMessages.push(message)
+      originalError(message)
+    })
+  })
+
+  afterEach(() => {
+    console.error = originalError
+  })
+
+  it('should reproduce inkscape attribute errors when rendering OpikIconBig', () => {
+    console.log('\n=== TESTING OpikIconBig SVG ATTRIBUTE ERRORS ===')
+
+    // Test multiple renders to check for inconsistency
+    for (let i = 0; i < 5; i++) {
+      console.log(`\nRender attempt ${i + 1}:`)
+
+      const { unmount } = render(<OpikIconBig />)
+
+      // Check for specific inkscape attribute errors
+      const inkscapeErrors = errorMessages.filter(msg =>
+        typeof msg === 'string' && msg.includes('inkscape'),
+      )
+
+      if (inkscapeErrors.length > 0) {
+        console.log(`Found ${inkscapeErrors.length} inkscape errors:`)
+        inkscapeErrors.forEach((error, index) => {
+          console.log(`  ${index + 1}. ${error.substring(0, 100)}...`)
+        })
+      }
+ else {
+        console.log('No inkscape errors found in this render')
+      }
+
+      unmount()
+
+      // Clear errors for next iteration
+      errorMessages = []
+    }
+  })
+
+  it('should analyze the SVG structure causing the errors', () => {
+    console.log('\n=== ANALYZING SVG STRUCTURE ===')
+
+    // Import the JSON data directly
+    const iconData = require('@/app/components/base/icons/src/public/tracing/OpikIconBig.json')
+
+    console.log('Icon structure analysis:')
+    console.log('- Root element:', iconData.icon.name)
+    console.log('- Children count:', iconData.icon.children?.length || 0)
+
+    // Find problematic elements
+    const findProblematicElements = (node: any, path = '') => {
+      const problematicElements: any[] = []
+
+      if (node.name && (node.name.includes(':') || node.name.startsWith('sodipodi'))) {
+        problematicElements.push({
+          path,
+          name: node.name,
+          attributes: Object.keys(node.attributes || {}),
+        })
+      }
+
+      // Check attributes for inkscape/sodipodi properties
+      if (node.attributes) {
+        const problematicAttrs = Object.keys(node.attributes).filter(attr =>
+          attr.startsWith('inkscape:') || attr.startsWith('sodipodi:'),
+        )
+
+        if (problematicAttrs.length > 0) {
+          problematicElements.push({
+            path,
+            name: node.name,
+            problematicAttributes: problematicAttrs,
+          })
+        }
+      }
+
+      if (node.children) {
+        node.children.forEach((child: any, index: number) => {
+          problematicElements.push(
+            ...findProblematicElements(child, `${path}/${node.name}[${index}]`),
+          )
+        })
+      }
+
+      return problematicElements
+    }
+
+    const problematicElements = findProblematicElements(iconData.icon, 'root')
+
+    console.log(`\n🚨 Found ${problematicElements.length} problematic elements:`)
+    problematicElements.forEach((element, index) => {
+      console.log(`\n${index + 1}. Element: ${element.name}`)
+      console.log(`   Path: ${element.path}`)
+      if (element.problematicAttributes)
+        console.log(`   Problematic attributes: ${element.problematicAttributes.join(', ')}`)
+    })
+  })
+
+  it('should test the normalizeAttrs function behavior', () => {
+    console.log('\n=== TESTING normalizeAttrs FUNCTION ===')
+
+    const { normalizeAttrs } = require('@/app/components/base/icons/utils')
+
+    const testAttributes = {
+      'inkscape:showpageshadow': '2',
+      'inkscape:pageopacity': '0.0',
+      'inkscape:pagecheckerboard': '0',
+      'inkscape:deskcolor': '#d1d1d1',
+      'sodipodi:docname': 'opik-icon-big.svg',
+      'xmlns:inkscape': 'https://www.inkscape.org/namespaces/inkscape',
+      'xmlns:sodipodi': 'https://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd',
+      'xmlns:svg': 'https://www.w3.org/2000/svg',
+      'data-name': 'Layer 1',
+      'normal-attr': 'value',
+      'class': 'test-class',
+    }
+
+    console.log('Input attributes:', Object.keys(testAttributes))
+
+    const normalized = normalizeAttrs(testAttributes)
+
+    console.log('Normalized attributes:', Object.keys(normalized))
+    console.log('Normalized values:', normalized)
+
+    // Check if problematic attributes are still present
+    const problematicKeys = Object.keys(normalized).filter(key =>
+      key.toLowerCase().includes('inkscape') || key.toLowerCase().includes('sodipodi'),
+    )
+
+    if (problematicKeys.length > 0)
+      console.log(`🚨 PROBLEM: Still found problematic attributes: ${problematicKeys.join(', ')}`)
+     else
+      console.log('✅ No problematic attributes found after normalization')
+  })
+})
diff --git a/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/overview/tracing/config-button.tsx b/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/overview/tracing/config-button.tsx
index 3d05575127..1ab40e31bf 100644
--- a/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/overview/tracing/config-button.tsx
+++ b/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/overview/tracing/config-button.tsx
@@ -1,12 +1,9 @@
 'use client'
 import type { FC } from 'react'
-import React, { useCallback, useEffect, useRef, useState } from 'react'
-import {
-  RiEqualizer2Line,
-} from '@remixicon/react'
+import React, { useCallback, useRef, useState } from 'react'
+
 import type { PopupProps } from './config-popup'
 import ConfigPopup from './config-popup'
-import cn from '@/utils/classnames'
 import {
   PortalToFollowElem,
   PortalToFollowElemContent,
@@ -17,13 +14,13 @@ type Props = {
   readOnly: boolean
   className?: string
   hasConfigured: boolean
-  controlShowPopup?: number
+  children?: React.ReactNode
 } & PopupProps
 
 const ConfigBtn: FC<Props> = ({
   className,
   hasConfigured,
-  controlShowPopup,
+  children,
   ...popupProps
 }) => {
   const [open, doSetOpen] = useState(false)
@@ -37,13 +34,6 @@ const ConfigBtn: FC<Props> = ({
     setOpen(!openRef.current)
   }, [setOpen])
 
-  useEffect(() => {
-    if (controlShowPopup)
-      // setOpen(!openRef.current)
-      setOpen(true)
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [controlShowPopup])
-
   if (popupProps.readOnly && !hasConfigured)
     return null
 
@@ -52,14 +42,11 @@ const ConfigBtn: FC<Props> = ({
       open={open}
       onOpenChange={setOpen}
       placement='bottom-end'
-      offset={{
-        mainAxis: 12,
-        crossAxis: hasConfigured ? 8 : 49,
-      }}
+      offset={12}
     >
       <PortalToFollowElemTrigger onClick={handleTrigger}>
-        <div className={cn(className, 'rounded-md p-1')}>
-          <RiEqualizer2Line className='h-4 w-4 text-text-tertiary' />
+        <div className="select-none">
+          {children}
         </div>
       </PortalToFollowElemTrigger>
       <PortalToFollowElemContent className='z-[11]'>
diff --git a/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/overview/tracing/panel.tsx b/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/overview/tracing/panel.tsx
index d082523222..7564a0f3c8 100644
--- a/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/overview/tracing/panel.tsx
+++ b/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/overview/tracing/panel.tsx
@@ -1,8 +1,9 @@
 'use client'
 import type { FC } from 'react'
-import React, { useCallback, useEffect, useState } from 'react'
+import React, { useEffect, useState } from 'react'
 import {
   RiArrowDownDoubleLine,
+  RiEqualizer2Line,
 } from '@remixicon/react'
 import { useTranslation } from 'react-i18next'
 import { usePathname } from 'next/navigation'
@@ -180,10 +181,6 @@ const Panel: FC = () => {
     })()
   }, [])
 
-  const [controlShowPopup, setControlShowPopup] = useState<number>(0)
-  const showPopup = useCallback(() => {
-    setControlShowPopup(Date.now())
-  }, [setControlShowPopup])
   if (!isLoaded) {
     return (
       <div className='mb-3 flex items-center justify-between'>
@@ -196,46 +193,66 @@ const Panel: FC = () => {
 
   return (
     <div className={cn('flex items-center justify-between')}>
-      <div
-        className={cn(
-          'flex cursor-pointer items-center rounded-xl border-l-[0.5px] border-t border-effects-highlight bg-background-default-dodge p-2 shadow-xs hover:border-effects-highlight-lightmode-off hover:bg-background-default-lighter',
-          controlShowPopup && 'border-effects-highlight-lightmode-off bg-background-default-lighter',
-        )}
-        onClick={showPopup}
-      >
-        {!inUseTracingProvider && (
-          <>
+      {!inUseTracingProvider && (
+        <ConfigButton
+          appId={appId}
+          readOnly={readOnly}
+          hasConfigured={false}
+          enabled={enabled}
+          onStatusChange={handleTracingEnabledChange}
+          chosenProvider={inUseTracingProvider}
+          onChooseProvider={handleChooseProvider}
+          arizeConfig={arizeConfig}
+          phoenixConfig={phoenixConfig}
+          langSmithConfig={langSmithConfig}
+          langFuseConfig={langFuseConfig}
+          opikConfig={opikConfig}
+          weaveConfig={weaveConfig}
+          aliyunConfig={aliyunConfig}
+          onConfigUpdated={handleTracingConfigUpdated}
+          onConfigRemoved={handleTracingConfigRemoved}
+        >
+          <div
+            className={cn(
+              'flex cursor-pointer select-none items-center rounded-xl border-l-[0.5px] border-t border-effects-highlight bg-background-default-dodge p-2 shadow-xs hover:border-effects-highlight-lightmode-off hover:bg-background-default-lighter',
+            )}
+          >
             <TracingIcon size='md' />
             <div className='system-sm-semibold mx-2 text-text-secondary'>{t(`${I18N_PREFIX}.title`)}</div>
-            <div className='flex items-center' onClick={e => e.stopPropagation()}>
-              <ConfigButton
-                appId={appId}
-                readOnly={readOnly}
-                hasConfigured={false}
-                enabled={enabled}
-                onStatusChange={handleTracingEnabledChange}
-                chosenProvider={inUseTracingProvider}
-                onChooseProvider={handleChooseProvider}
-                arizeConfig={arizeConfig}
-                phoenixConfig={phoenixConfig}
-                langSmithConfig={langSmithConfig}
-                langFuseConfig={langFuseConfig}
-                opikConfig={opikConfig}
-                weaveConfig={weaveConfig}
-                aliyunConfig={aliyunConfig}
-                onConfigUpdated={handleTracingConfigUpdated}
-                onConfigRemoved={handleTracingConfigRemoved}
-                controlShowPopup={controlShowPopup}
-              />
+            <div className='rounded-md p-1'>
+              <RiEqualizer2Line className='h-4 w-4 text-text-tertiary' />
             </div>
             <Divider type='vertical' className='h-3.5' />
             <div className='rounded-md p-1'>
               <RiArrowDownDoubleLine className='h-4 w-4 text-text-tertiary' />
             </div>
-          </>
-        )}
-        {hasConfiguredTracing && (
-          <>
+          </div>
+        </ConfigButton>
+      )}
+      {hasConfiguredTracing && (
+        <ConfigButton
+          appId={appId}
+          readOnly={readOnly}
+          hasConfigured
+          enabled={enabled}
+          onStatusChange={handleTracingEnabledChange}
+          chosenProvider={inUseTracingProvider}
+          onChooseProvider={handleChooseProvider}
+          arizeConfig={arizeConfig}
+          phoenixConfig={phoenixConfig}
+          langSmithConfig={langSmithConfig}
+          langFuseConfig={langFuseConfig}
+          opikConfig={opikConfig}
+          weaveConfig={weaveConfig}
+          aliyunConfig={aliyunConfig}
+          onConfigUpdated={handleTracingConfigUpdated}
+          onConfigRemoved={handleTracingConfigRemoved}
+        >
+          <div
+            className={cn(
+              'flex cursor-pointer select-none items-center rounded-xl border-l-[0.5px] border-t border-effects-highlight bg-background-default-dodge p-2 shadow-xs hover:border-effects-highlight-lightmode-off hover:bg-background-default-lighter',
+            )}
+          >
             <div className='ml-4 mr-1 flex items-center'>
               <Indicator color={enabled ? 'green' : 'gray'} />
               <div className='system-xs-semibold-uppercase ml-1.5 text-text-tertiary'>
@@ -243,33 +260,14 @@ const Panel: FC = () => {
               </div>
             </div>
             {InUseProviderIcon && <InUseProviderIcon className='ml-1 h-4' />}
-            <Divider type='vertical' className='h-3.5' />
-            <div className='flex items-center' onClick={e => e.stopPropagation()}>
-              <ConfigButton
-                appId={appId}
-                readOnly={readOnly}
-                hasConfigured
-                className='ml-2'
-                enabled={enabled}
-                onStatusChange={handleTracingEnabledChange}
-                chosenProvider={inUseTracingProvider}
-                onChooseProvider={handleChooseProvider}
-                arizeConfig={arizeConfig}
-                phoenixConfig={phoenixConfig}
-                langSmithConfig={langSmithConfig}
-                langFuseConfig={langFuseConfig}
-                opikConfig={opikConfig}
-                weaveConfig={weaveConfig}
-                aliyunConfig={aliyunConfig}
-                onConfigUpdated={handleTracingConfigUpdated}
-                onConfigRemoved={handleTracingConfigRemoved}
-                controlShowPopup={controlShowPopup}
-              />
+            <div className='ml-2 rounded-md p-1'>
+              <RiEqualizer2Line className='h-4 w-4 text-text-tertiary' />
             </div>
-          </>
-        )}
-      </div >
-    </div >
+            <Divider type='vertical' className='h-3.5' />
+          </div>
+        </ConfigButton>
+      )}
+    </div>
   )
 }
 export default React.memo(Panel)
diff --git a/web/app/components/apps/footer.tsx b/web/app/components/apps/footer.tsx
index 18b7779651..c5efb2b8b4 100644
--- a/web/app/components/apps/footer.tsx
+++ b/web/app/components/apps/footer.tsx
@@ -36,13 +36,13 @@ const Footer = () => {
     return null
 
   return (
-    <footer className='shrink-0 grow-0 px-12 py-2 relative'>
+    <footer className='relative shrink-0 grow-0 px-12 py-2'>
       <button
         onClick={handleClose}
-        className='absolute top-2 right-2 flex h-6 w-6 cursor-pointer items-center justify-center rounded-full transition-colors duration-200 ease-in-out hover:bg-gray-100 dark:hover:bg-gray-800'
+        className='absolute right-2 top-2 flex h-6 w-6 cursor-pointer items-center justify-center rounded-full transition-colors duration-200 ease-in-out hover:bg-components-main-nav-nav-button-bg-active'
         aria-label="Close footer"
       >
-        <RiCloseLine className='h-4 w-4 text-text-tertiary' />
+        <RiCloseLine className='h-4 w-4 text-text-tertiary hover:text-text-secondary' />
       </button>
       <h3 className='text-gradient text-xl font-semibold leading-tight'>{t('app.join')}</h3>
       <p className='system-sm-regular mt-1 text-text-tertiary'>{t('app.communityIntro')}</p>
diff --git a/web/app/components/base/icons/utils.ts b/web/app/components/base/icons/utils.ts
index 90d075f01c..632e362075 100644
--- a/web/app/components/base/icons/utils.ts
+++ b/web/app/components/base/icons/utils.ts
@@ -14,9 +14,26 @@ export type Attrs = {
 
 export function normalizeAttrs(attrs: Attrs = {}): Attrs {
   return Object.keys(attrs).reduce((acc: Attrs, key) => {
+    // Filter out editor metadata attributes before processing
+    if (key.startsWith('inkscape:')
+        || key.startsWith('sodipodi:')
+        || key.startsWith('xmlns:inkscape')
+        || key.startsWith('xmlns:sodipodi')
+        || key.startsWith('xmlns:svg')
+        || key === 'data-name')
+      return acc
+
     const val = attrs[key]
     key = key.replace(/([-]\w)/g, (g: string) => g[1].toUpperCase())
     key = key.replace(/([:]\w)/g, (g: string) => g[1].toUpperCase())
+
+    // Additional filter after camelCase conversion
+    if (key === 'xmlnsInkscape'
+        || key === 'xmlnsSodipodi'
+        || key === 'xmlnsSvg'
+        || key === 'dataName')
+      return acc
+
     switch (key) {
       case 'class':
         acc.className = val
diff --git a/web/app/components/base/tag-management/filter.tsx b/web/app/components/base/tag-management/filter.tsx
index ecc159b2fc..4cf01fdc26 100644
--- a/web/app/components/base/tag-management/filter.tsx
+++ b/web/app/components/base/tag-management/filter.tsx
@@ -139,7 +139,10 @@ const TagFilter: FC<TagFilterProps> = ({
             </div>
             <div className='border-t-[0.5px] border-divider-regular' />
             <div className='p-1'>
-              <div className='flex cursor-pointer items-center gap-2 rounded-lg py-[6px] pl-3 pr-2 hover:bg-state-base-hover' onClick={() => setShowTagManagementModal(true)}>
+              <div className='flex cursor-pointer items-center gap-2 rounded-lg py-[6px] pl-3 pr-2 hover:bg-state-base-hover' onClick={() => {
+                setShowTagManagementModal(true)
+                setOpen(false)
+              }}>
                 <Tag03 className='h-4 w-4 text-text-tertiary' />
                 <div className='grow truncate text-sm leading-5 text-text-secondary'>
                   {t('common.tag.manageTags')}