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

Add terminal grouping support #1518

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Lancelotbronner
Copy link

@Lancelotbronner Lancelotbronner commented Dec 26, 2023

Description

This is still a draft, it's (mostly) working but needs polish and bugfixing.
Here are the refactors & features:

  • Refactored terminal state from being spread out over a few views/models to being aggregated into a single TerminalEmulator model, simplified a few views (this was required for multiple simultaneous shells)
  • Added a TerminalGroup model to represented grouped terminals
  • Drag & drop one or more terminals onto an ungrouped one to create a group
  • Drag & drop one or more terminals onto a group to move them into it
  • Freely rename groups and terminals (or leave them automatic by removing your custom title)

Related Issues

Checklist

  • Bugfix: moving sometimes flattens the disclosure groups into the list (SwiftUI???)
  • Cleanup the models (I'd like some feedback on it first)
  • Let groups be part of drag & drop operations
  • Ensure each terminals get their own "mini-toolbar"
  • Cleanup the views (I know its hacked together, I'd like to have the model feedback first)

  • Wishlist: insert terminals at a specific spot between sibling groups (maybe a SwiftUI limitation)
  • Wishlist: click to focus in the terminal area (maybe a SwiftTerm limitation)

  • I read and understood the contributing guide as well as the code of conduct
  • The issues this PR addresses are related to each other
  • My changes generate no new warnings
  • My code builds and runs on my machine
  • My changes are all related to the related issue above
  • I documented my code

Screenshots

… to use groups

- List reordering issue
- Drag & drop instead of move is weird
- Could embed instead so onMove would do all the merging work, drag & drop would only be on single-item groups
@activcoding
Copy link
Member

Just wanted to throw this gem in here, I found it in Figma a few weeks ago and almost forgot to share it: Screenshot 2024-01-09 at 1 34 53 PM

@austincondiff
Copy link
Collaborator

@activcoding Yeah, that was definitely an experimental design. There are a few concerns I had with it so I ended up moving on.

@Lancelotbronner
Copy link
Author

Lancelotbronner commented Jan 19, 2024

Personally I don't really like it so I'm glad it's gone but that does give me an idea.

I'm having issues with DisclosureGroup messing with onMove indexes and preventing it to work as intended and it might be fixable by switching the design a bit.

I'll implement it (a List using Section for groups) and post a screenshot back see what people think of it.

Update: It doesn't work, turns out it's ForEach that messes with the indexes, I'm not sure what the solution is but it would also improve the behaviour of the files outline on the sidebar.

Maybe we should file a radar for OutlineGroup to conform to DynamicViewContent.

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.

None yet

3 participants