Conversation
|
Is there a reason for the |
| struct InspectorSidebar: View { | ||
| @ObservedObject var workspace: WorkspaceDocument | ||
| var windowController: NSWindowController | ||
| @State private var selection: Int = 0 |
There was a problem hiding this comment.
we should try to avoid using types like Int or String etc, which are really generic types.
Here we could create a new enum, I think the logic will end up being much cleaner and easier to read
There was a problem hiding this comment.
I copied what was done in the navigator sidebar. I do agree though. Although it might end up having to be somewhat dynamic as we allow extensions to add new sidebar tabs.
|
|
||
| var body: some View { | ||
| HStack(spacing: 10) { | ||
| icon(systemImage: "doc", title: "File Inspector", id: 0) |
There was a problem hiding this comment.
if we use that enum I was talking about here it would end up being like:
icon(systemImage: "doc", title: "File Inspector", id: .fileInspector)
| workspace: workspace) | ||
| .frame(maxWidth: .infinity, maxHeight: .infinity) | ||
| InspectorSidebar(workspace: workspace, windowController: windowController) | ||
| .frame(minWidth: 250, maxWidth: .infinity, maxHeight: .infinity) |
There was a problem hiding this comment.
using fixed values worries me a little bit... maybe 250 it's low and not that much but we should definitely put this value inside a constant that maybe is injected upon initalization?
MarcoCarnevali
left a comment
MarcoCarnevali
left a comment
There was a problem hiding this comment.
One thing I would do before merging this is creating a module for the InspectorSidebar and a SnapshotTest
|
There are other files that need to be modules as well that are not. This was meant to just be a placeholder for later. I'd like to create a separate issue to move other things into modules as well like NavigationSidebar, Breadcrumbs, Statusbar etc. Doing this work now is out of scope of this PR. I will merge this in and we can always revise later. |
# Description This PR adds a `contentInsets` parameter that lets users set an inset object for the editor. It also updates `STTextView` to be able to set the left inset correctly. When left `nil` the scroll view will use automatic insets. This also doesn't override the overscroll option, but rather adds the overscroll after the insets are applied. # Related Issues - [Discord discussion](https://canary.discord.com/channels/951544472238444645/954084547694325861/1075085722211581952) - [STTextView commit](krzyzanowskim/STTextView@06d8f33) # Screenshots Adding top and left insets: <img width="741" alt="Screenshot 2023-02-14 at 3 09 57 PM" src="https://user-images.githubusercontent.com/35942988/218863914-8869e185-cd21-45a7-a7aa-58387ea778ea.png"> Adding left and bottom insets (scroller is also inset): <img width="835" alt="Screenshot 2023-02-14 at 3 10 15 PM" src="https://user-images.githubusercontent.com/35942988/218863899-99d2b124-fc96-418a-9869-d6897a7baea4.png">
3 participants
No description provided.