fix: Multiple UI component improvements and code quality enhancements (#23446)

2025-08-10 10:18:06 +00:00 · 2025-08-05 19:36:07 +08:00 · 2025-08-05 19:36:07 +08:00 · 2cd3fe0dce
commit 2cd3fe0dce
parent 5eb061466f
6 changed files with 249 additions and 88 deletions
--- 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
@ -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')
+  })
+})
--- 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]'>
--- 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)
--- 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>
--- 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
--- 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')}