Skip to content

feat: add connections group#76

Merged
debba merged 10 commits intodebba:mainfrom
fandujar:main
Mar 9, 2026
Merged

feat: add connections group#76
debba merged 10 commits intodebba:mainfrom
fandujar:main

Conversation

@fandujar
Copy link
Contributor

@fandujar fandujar commented Mar 8, 2026 •
edited
Loading

PR Description

Feature: Add connections group

Screenhots:
Screenshot 2026-03-08 at 00 57 30

Screenshot 2026-03-08 at 00 57 47 Screenshot 2026-03-08 at 00 57 39

kilo-code-bot[bot]
kilo-code-bot bot reviewed Mar 8, 2026
View reviewed changes
@kilo-code-bot
Copy link

kilo-code-bot bot commented Mar 8, 2026 •
edited
Loading

Code Review Summary

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0

Summary

This PR adds connection grouping functionality with collapsible folders, drag-and-drop support, and group management. While the grid view was fully updated to support groups, the list view was not updated and will not display the group structure.

Issue Details (click to expand)

WARNING

File Line Issue
src/pages/Connections.tsx N/A List view doesn't show groups — The list view still uses the flat filtered array instead of rendering grouped connections. When users switch to list view, the group organization is lost.
Other Observations (not in diff)

  1. List view inconsistency (lines ~821-900): The list view rendering code was not updated to support groups. It still iterates over filtered.map() instead of showing grouped connections like the grid view does. Consider either:

    • Updating list view to show groups similar to grid view
    • Disabling list view when groups exist
    • Adding a note that list view doesn't support groups

  2. Existing issues already flagged (from previous review):

    • Stale data source in loadConnections
    • Various floating promises in async handlers
    • onBlur + Escape key race condition in rename inputs
    • Performance concern with load_connections_file
Files Reviewed (11 files)
  • src/pages/Connections.tsx — Grid view updated with groups, list view unchanged
  • src/contexts/DatabaseContext.ts — Added ConnectionGroup interface and group methods
  • src/contexts/DatabaseProvider.tsx — Added group management functions
  • src/contexts/EditorProvider.tsx — Fixed tab merging for multiple connections
  • src/components/layout/sidebar/ConnectionGroupFolder.tsx — New component for sidebar groups
  • src/components/layout/Sidebar.tsx — Updated to use groups
  • src/components/ui/NewConnectionModal.tsx — Updated for group_id/sort_order
  • src/i18n/locales/en.json — Added group translation keys
  • src/utils/tabFilters.ts — Fixed getActiveTabForConnection fallback
  • src-tauri/src/persistence.rs — Added group persistence
  • src-tauri/src/commands.rs — Added group commands
  • package-lock.json — Added lockfile

Fix these issues in Kilo Cloud

kilo-code-bot[bot]
kilo-code-bot bot reviewed Mar 8, 2026
View reviewed changes
kilo-code-bot[bot]
kilo-code-bot bot reviewed Mar 8, 2026
View reviewed changes
kilo-code-bot[bot]
kilo-code-bot bot reviewed Mar 8, 2026
View reviewed changes
@debba debba self-assigned this Mar 8, 2026
@debba
Copy link
Owner

debba commented Mar 8, 2026

@fandujar
Thanks for your contribution.
I tried your work, but it looks like there are some issues.

For example, if you move connections between groups, you’ll notice that after moving them some connections stop working and show a “Connection not found” error. This does not happen on the main branch.

I also noticed some issues with the split screen in the lateral sidebar. You can reproduce them by selecting multiple connections with Ctrl + click and then splitting the screen after the selection.

By the way, the feature is very interesting, and it would be great if these issues could be fixed.

I would also suggest removing the “Remove from Group” option when the group is “Ungrouped.”

@fandujar
Copy link
Contributor Author

fandujar commented Mar 8, 2026

@debba, I think I managed to fix all issues. Let me know if you find something else.

debba
debba reviewed Mar 9, 2026
View reviewed changes
Copy link
Owner

@debba debba left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I analyzed the PR and found a performance issue in the persistence module: load_connections_file reads the keychain for all saved connections every time it is called. Since this function is invoked on every DB operation (connect, query, schema, tables...) via find_connection_by_id, this generates N keychain reads per single operation, where N is the number of saved connections.

The suggestion proposes making load_connections_file a raw function (JSON parsing only, no keychain access). find_connection_by_id already fetches the password only for the specific connection it finds, so no logic is lost.

@debba
Copy link
Owner

debba commented Mar 9, 2026

@fandujar yes it's working now, but I suggested you some changes to do, because now for each operation unlock password to all connections.
You can directly merge from here, if you want: https://github.com/debba/tabularis/tree/fandujar/main

@fandujar
Copy link
Contributor Author

fandujar commented Mar 9, 2026

Hi @debba Thanks for finding this issue and fixing it. Can you check my PR again, please?

@debba debba merged commit 6f48a87 into debba:main Mar 9, 2026
@debba
Copy link
Owner

debba commented Mar 9, 2026

Merged. Thanks a lot for your contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@debba debba debba left review comments

@kilo-code-bot kilo-code-bot[bot] kilo-code-bot[bot] left review comments

Assignees

@debba debba

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@fandujar @debba