Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: reorganize top level widgets tab #406

Merged
merged 2 commits into from
Feb 4, 2025
Merged

Conversation

Charlie-XIAO
Copy link
Contributor

@Charlie-XIAO Charlie-XIAO commented Feb 4, 2025

Extracted from and refactored upon #370. This PR mainly does the following:

  • Group components by tab. Each tab is now a subfolder of the components directory with index.tsx being the tab component itself and other files being relevant building blocks.
  • Memoize each tab.
  • Make Widgets tab care only about IDs. The building blocks Trigger, Config and Settings are memoized and care about only one of config and settings of specific widget as they need. They are memoized as well.
  • The widgets store is made back into Record<string, { config: WidgetConfig, settings: WidgetSettings } instead of { id: string, config: WidgetConfig, settings: WidgetSettings }[]. This is so that we can access each widget by ID (which is reliable) instead of by index (not sure if there will be issues). If by ID but using array, we would either need to make parents pass stuff down which can cause unnecessary rerendering in parents, or we would need to use find which seems not performant. Switching back to Record we can still ensure sortedness by creating object from sorted entries.

Future work include:

  • Each of Trigger, Config, and Settings.
  • Other tabs.

@Charlie-XIAO Charlie-XIAO merged commit c9725ec into main Feb 4, 2025
11 checks passed
@Charlie-XIAO Charlie-XIAO deleted the refactor/widgets-tab branch February 4, 2025 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant