Skip to content

Conversation

Malay-dev
Copy link

Description

Refactored the WorkflowNodesTabs component to eliminate code duplication by generalizing four nearly-identical TabsContent blocks into a single reusable rendering function.

Changes made:

  • Created a TabConfigProps interface to define tab configuration structure
  • Introduced a tabConfigs array using useMemo that centralizes all tab configurations
  • Extracted a renderTabContent function that handles the common rendering logic
  • Reduced ~200 lines of duplicated code into a single maintainable implementation

Benefits:

  • Follows DRY (Don't Repeat Yourself) principle
  • Easier to maintain - changes only need to be made once
  • Improved code readability and clarity
  • No functional changes - all existing behavior preserved

Fixes #2227

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • New connector (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

  • Verified all four tabs (Triggers, Actions, Flows, Cluster Elements) render correctly
  • Tested conditional rendering based on hide props (hideActionComponents, hideTriggerComponents, etc.)
  • Verified empty state messages display appropriately for each tab
  • Tested drag-and-drop functionality with itemsDraggable prop
  • Verified onClick handlers work correctly for all tab types
  • Tested filter functionality on Actions tab
  • Verified selected state highlighting on components tab
  • Confirmed no TypeScript errors or ESLint warnings
  • All existing unit tests pass

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Copy link

gitguardian bot commented Oct 5, 2025

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
While these secrets were previously flagged, we no longer have a reference to the
specific commits where they were detected. Once a secret has been leaked into a git
repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@kresimir-coko
Copy link
Collaborator

Can you resend your code in this PR without the merge commit please @Malay-dev . We use rebasing here in Bytechef, so we want to avoid merge commits.

Copy link
Collaborator

@kresimir-coko kresimir-coko left a comment

Choose a reason for hiding this comment

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

Good job @Malay-dev, while your solution does work, it has some problems:

  • Anti-pattern: Function-based rendering - The renderTabContent function violates React best practices
  • Hard to test - Function-based rendering is difficult to unit test in isolation
  • Performance concerns - Function recreated on every render
  • Accessibility issues - The function approach makes it harder to implement proper ARIA patterns

If you want to try to improve it, rewrite it using a component-based approach - don't use a function to render the tab content (replace renderTabContent with <TabContent />).

Also, magic numbers are a code smell (tabConfigs[magicNumber]) - we should use a string (maybe matching the tabConfig.value) or something else that is very clear when reading the code as someone who hasn't written it.

@Malay-dev Malay-dev force-pushed the 2227-Refactor_WorkflowNodesTabs branch 2 times, most recently from ff74a4f to 23bde4b Compare October 6, 2025 17:00
@Malay-dev Malay-dev force-pushed the 2227-Refactor_WorkflowNodesTabs branch from 23bde4b to 9899943 Compare October 6, 2025 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[task] Generalize TabsContent in WorkflowNodesTabs to reduce duplication
2 participants