Skip to content

chore(ui): handle description render common with util#24792

Merged
chirag-madlani merged 17 commits intomainfrom
chore-description-table
Dec 18, 2025
Merged

chore(ui): handle description render common with util#24792
chirag-madlani merged 17 commits intomainfrom
chore-description-table

Conversation

@chirag-madlani
Copy link
Copy Markdown
Collaborator

@chirag-madlani chirag-madlani commented Dec 11, 2025

Describe your changes:

Fixes: Remove unwanted description. Trim from all the occurrences and let the component handles it gracefully
Since we switched to HTML based description, we will always have description as <p></p> or a similar patter for an empty line
Enhance utility function to handle all of those cases and render either no description or actual content based on the condition meet

I worked on ... because ...

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

  • New utility function:
    • descriptionTableObject<T>() in TableColumn.util.tsx centralizes description column configuration across 60+ table components
  • Enhanced empty content detection:
    • isDescriptionContentEmpty() in BlockEditorUtils.ts now handles HTML patterns like <p></p>, <p> </p>, <p>\n</p>, and unicode non-breaking spaces
  • Component-level empty state:
    • RichTextEditorPreviewerNew now checks for empty content and displays "No description" placeholder automatically
  • Refactored 60+ components:
    • Replaced duplicated description rendering logic in Container, Dashboard, Drive, Database, Settings, Glossary, and Service components
  • Comprehensive test coverage:
    • Added 1,300+ lines of tests including TableColumn.util.test.tsx and enhanced BlockEditorUtils.test.ts with 400+ edge case scenarios

This will update automatically on new commits.


Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR consolidates duplicate description column rendering logic across the codebase by introducing a reusable descriptionTableObject utility function. It also enhances the isDescriptionContentEmpty function to properly detect empty HTML content patterns like <p></p>, <p> </p>, and <p>\u00A0</p> that result from empty rich text editors. The changes reduce code duplication and provide consistent description rendering behavior across 27+ components.

Key Changes:

  • Created descriptionTableObject utility function in TableColumn.util.tsx that generates standardized description column configurations
  • Enhanced isDescriptionContentEmpty function with regex-based detection of empty HTML paragraph tags, including those with whitespace and unicode non-breaking spaces
  • Added comprehensive test suite with 400+ test cases for the enhanced empty content detection logic

Reviewed changes

Copilot reviewed 35 out of 35 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
TableColumn.util.tsx Added descriptionTableObject utility function to generate description column configurations; integrates with RichTextEditorPreviewerNew component
BlockEditorUtils.ts Enhanced isDescriptionContentEmpty with regex to detect empty HTML paragraphs; added console.warn for error handling
BlockEditorUtils.test.ts Added comprehensive test suite covering null/undefined, empty paragraphs, whitespace, unicode characters, HTML entities, and edge cases
ServiceMainTabContentUtils.tsx Replaced inline description column with utility; improved type safety by replacing any with Operation[] type
Database.util.tsx Replaced inline description column with utility; removed unused viewAllPermission parameter from function signature
ClassificationUtils.tsx Replaced inline description column with utility
RichTextEditor.interface.ts Made markdown prop optional in PreviewerProp interface to support undefined descriptions
StoredProcedureTab.tsx Replaced inline description column with utility
RolesListPage.tsx Replaced inline description column with utility; removed unused NO_DATA_PLACEHOLDER import
RolesDetailPageList.component.tsx Replaced inline description column with utility
PoliciesListPage.tsx Replaced inline description column with utility; removed unused NO_DATA_PLACEHOLDER import
PoliciesDetailsList.component.tsx Replaced inline description column with utility
ObservabilityAlertsPage.tsx Replaced inline description column with utility; removed unused isEmpty import
NotificationListPage.tsx Replaced inline description column with utility; removed unused isEmpty import
MetricListPage.tsx Replaced inline description column with utility; removed unused isEmpty import
SchemaTablesTab.tsx Replaced inline description column with utility
KPIList.tsx Replaced inline description column with utility
APIEndpointsTab.tsx Replaced inline description column with utility
TeamHierarchy.tsx Replaced inline description column with utility; removed unused imports
RolesAndPoliciesList.tsx Replaced inline description column with utility
Services.tsx Replaced inline description column with utility; changed column width from 200 to 400 pixels
CustomPropertyTable.tsx Replaced inline description column with utility
BotListV1.component.tsx Replaced inline description column with utility
PipelineVersion.component.tsx Replaced inline description column with utility
WorkflowsTable.tsx Replaced inline description column with utility; removed unused Typography import
SpreadsheetsTable.tsx Replaced inline description column with utility; removed unused isUndefined import
SpreadsheetVersion.tsx Replaced inline description column with utility
FilesTable.tsx Replaced inline description column with utility; removed unused isUndefined import
DirectoryVersion.tsx Replaced inline description column with utility
DirectoryChildrenTable.tsx Replaced inline description column with utility
ContainerChildren.tsx Replaced inline description column with utility; extracted interface to separate file
ContainerChildren.interface.ts New file containing extracted ContainerChildrenProps interface
DatabaseSchemaTable.tsx Replaced inline description column with utility
DataModelsTable.tsx Replaced inline description column with utility; removed unused isUndefined import
DashboardVersion.component.tsx Replaced inline description column with utility

Comment thread openmetadata-ui/src/main/resources/ui/src/utils/BlockEditorUtils.ts
Comment thread openmetadata-ui/src/main/resources/ui/src/utils/BlockEditorUtils.ts Outdated
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 17, 2025

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 64%
64.28% (50819/79056) 41.94% (24786/59101) 45.36% (7797/17189)

@gitar-bot
Copy link
Copy Markdown

gitar-bot Bot commented Dec 18, 2025

🔍 CI failure analysis for ffe34de: Playwright tests timing out due to loading spinners that never disappear after certification operations on Directory and File entities

Issue

Playwright CI test failures in job playwright-ci-postgresql (5, 6) with multiple test timeouts.

Root Cause

The tests are timing out because a loading spinner ([data-testid="loader"]) remains visible and never disappears. The test expects the loader count to be 0 but it stays at 1 after waiting for the full 30-second timeout period.

Specific failures:

  1. Directory › Certification Add Remove - Loader stuck, expected 0 loaders but found 1
  2. File › Certification Add Remove - Same issue, loader never disappears
  3. Worksheet › Glossary Term Add Update and Remove - Timeout while waiting for loader to disappear
  4. Table › UpVote/DownVote entity - Page/context/browser closed unexpectedly
  5. Table › Follow/Un-follow entity - Page/context/browser closed unexpectedly

Details

The pattern shows that certification-related operations (Add/Remove certifications) on Directory and File entities are causing the UI to enter a stuck loading state. The loader element is detected 33 times during the 30-second wait period, indicating the component is continuously rendering in a loading state without completing the operation.

This issue is likely related to the PR's changes to description rendering, as the refactoring touches table components across Container, Dashboard, Drive, Database, Settings, and other areas. The descriptionTableObject utility and empty content detection changes in RichTextEditorPreviewerNew may be affecting how these entity pages render or update their state during certification operations.

Potential causes:

  • The new descriptionTableObject utility may have introduced a rendering loop or state update issue
  • Empty content detection in isDescriptionContentEmpty might be causing repeated re-renders
  • The centralized description rendering may not be properly handling loading states for certification updates
Code Review 👍 Approved with suggestions

Solid refactor that centralizes description column handling with comprehensive test coverage. A few minor edge case and consistency items to consider.

Suggestions 💡 3 suggestions
Edge Case: descriptionTableObject render doesn't handle undefined text

📄 openmetadata-ui/src/main/resources/ui/src/utils/TableColumn.util.tsx:117

The new descriptionTableObject function's render method passes text directly to RichTextEditorPreviewerNew without handling the case where text might be undefined. While this may work because the markdown prop was changed to optional in RichTextEditor.interface.ts, the previous inline implementations often had explicit empty-state handling.

Looking at the RichTextEditorPreviewerNew component (based on test cases), it does appear to handle empty/undefined markdown by showing "label.no-description". However, to be explicit and consistent with TypeScript's strictness:

render: (text: string) => <RichTextEditorPreviewerNew markdown={text} />,

Could be more defensive as:

render: (text?: string) => <RichTextEditorPreviewerNew markdown={text ?? ''} />,

This is a minor issue since the component likely handles it, but explicit handling would be safer.

Code Quality: BotListV1 inconsistent with other refactored components

📄 openmetadata-ui/src/main/resources/ui/src/components/Settings/Bot/BotListV1/BotListV1.component.tsx:148

The BotListV1 component was updated but still has custom description column rendering with highlightSearchText, while all other components were refactored to use the centralized descriptionTableObject. This creates inconsistency:

render: (_, record) => (
  <RichTextEditorPreviewerNew
    markdown={highlightSearchText(
      record?.description || '',
      searchTerm
    )}
  />
),

This is intentional because of the highlightSearchText functionality, but it's worth noting that this component now differs from the pattern established elsewhere. Consider whether descriptionTableObject could accept a custom markdown transformer function to maintain consistency while supporting search highlighting use cases.

Code Quality: Added console.warn in isHTMLString catch block

📄 openmetadata-ui/src/main/resources/ui/src/utils/BlockEditorUtils.ts:196

A console.warn statement was added in the catch block of isHTMLString:

} catch (e) {
  // eslint-disable-next-line no-console
  console.warn('Error parsing content to check HTML string:', e);

  return false;
}

While debugging information can be useful, this will produce console output in production for any parsing errors. Consider:

  1. Using a proper logging utility if the project has one
  2. Only logging in development mode (process.env.NODE_ENV === 'development')
  3. Removing the console statement entirely since the function gracefully returns false anyway

The eslint-disable comment also indicates this may not be the intended pattern for the codebase.

What Works Well

  • Excellent code consolidation: The new descriptionTableObject utility eliminates significant code duplication across 30+ components
  • Comprehensive test coverage: Added extensive tests for RichTextEditorPreviewNew, RichTextEditorPreviewerV1, TaskDescriptionPreviewer, BlockEditorUtils, and TableColumn.util, with excellent edge case coverage
  • Clean pattern consistency: The new utility follows the established pattern of ownerTableObject, domainTableObject, tagTableObject, and dataProductTableObject
  • Improved isDescriptionContentEmpty logic: The new implementation handles more edge cases including whitespace, unicode nbsp characters, and empty paragraph tags with attributes
  • Type safety improvements: The descriptionTableObject function properly uses TypeScript generics and accepts additional column configuration via ColumnType<T>

Recommendations

  • Consider making the render function in descriptionTableObject more explicit about handling undefined values
  • The console.warn addition might produce noise in production - consider using a proper logging utility or environment-gating
  • The BotListV1 component uses a different pattern due to search highlighting - this is intentional but creates some inconsistency

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.
✅ Code review is on Gitar will review this change.
Display: compact Hiding non-applicable rules.

Comment with these commands to change:

Auto-apply ✅ Code review Compact
gitar auto-apply:on         
gitar code-review:off         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | This comment will update automatically (Docs)

@sonarqubecloud
Copy link
Copy Markdown

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 UI UI specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants