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

Manual data reload #14

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Manual data reload #14

wants to merge 7 commits into from

Conversation

RCCoop
Copy link
Contributor

@RCCoop RCCoop commented Apr 1, 2023

I ran into an issue where making edits to an attribute in the OutlineView needed to change properties of the provided TableViewCells, but no state change was causing the cells to update. So I put together this pull request which adds functionality for manually calling a version of NSOutlineView's reloadData or reloadItem.

The functionality requires two steps:

  • Set the reloadID of the OutlineView using the modifier function reloadIdentifier(_:)
  • If a manual reload is desired, call the static function triggerReloadOfOutlineView(id:itemIds:), with the OutlineView.reloadID set in the previous step as the first parameter, and an optional array of OutlineView.Data.Element.ID as the second parameter. If no element IDs are provided, the OutlineViewController calls reloadData, thus reloading the cell views for the entire view. If element IDs are provided, only the rows with the specified IDs will be reloaded.

The additional functionality is entirely optional, so won't require existing projects to be updated if not using reload functions.

I also added another sample project, showing where the functionality may be needed: In it, a color picker is used to provide the text color for FileItemView rows. Without the triggerReloadOfOutlineView functions, updating the binding value of ContentView.rootTextColor and ContentView.childTextColor would not immediately update the text color of the displayed cells. By calling the function during onChange(of: rootTextColor) and onChange(of: childTextColor), the text color updates as expected.

Screen.Recording.2023-04-01.at.10.51.39.AM.mov

@ZofiaBernadetta
Copy link
Collaborator

This is an excellent bug! By that I mean I hate everything about it already. 😄

I was thinking about it for some time and couldn't come up with a way to avoid calling reload ourselves. I am open to ideas for avoid the whole reload business of we can.

If we are stuck with reloading, Instead of providing context unaware global functions, I think we can have a slightly better API inspired from ScrollViewReader:

OutlineViewReader { proxy in
  OutlineView(...)
    .onChange(of: thatPeskyState) {
      proxy.reload()
      // or
      proxy.reload(items: itemsToReload) // This should probably accept a "sequence" of items 
    }
}

There are multiple ways of implementing the OutlineViewReader. One approach could be using an environment value:

public struct OutlineViewReader<Content: View>: View {
  @State private var proxy = OutlineViewReaderProxy()
  var content: (OutlineViewReaderProxy) -> Content

  var body: some View {
    content(proxy)
      .environment(\.outlineViewReaderProxy, proxy)
  }
}

The OutlineViewReaderProxy in this case would could be something like this:

public struct OutlineViewReaderProxy {
  enum ReloadState {
    case items([Any]) // Must be an erased type I suppose, because the OutlineViewReader does not know the element type.
    case all
    case none
  }

  let reloadState = CurrentValueSubject(ReloadState.none)

  public func reload() {
    reloadState.send(.all)
  }

  [...]
}

public extension OutlineViewReaderProxy: EnvironmentKey {
  static let defaultValue = OutlineViewReaderProxy()
}

public extension EnvironmentValues {
  var outlineViewReaderProxy: OutlineViewReaderProxy {
    get { self[OutlineViewReaderProxy.self] }
    set { self[OutlineViewReaderProxy.self] = newValue }
  }
}

Finally, in the OutlineView, we can use the environment object:

public struct OutlineView<...> {
  @Environment(\.outlineViewReaderProxy) var proxy

  public func updateNSViewController(
    _ outlineController: OutlineViewController<Data, Drop>,
    context: Context
  ) {
    [...]
    // The outlineController could have some logic for subscribing to the
    // publisher for reloads and performing the actual reload.
    // The proxy object may change during the runtime so need to make
    // sure that we are always subscribing to the latest object.
    outlineController.setProxy(proxy)
  }
}

@RCCoop
Copy link
Contributor Author

RCCoop commented Apr 14, 2023

Very cool idea! I like it... I haven't tried anything like it before, so I don't know what pitfalls there might be. But it is nice to not rely on Notifications and global functions.

In the meantime I have been using an alternate solution that also solves this issue that I found recently (although I don't think you'll like this solution too much 😆 ). My solution was to require that OutlineView.Data.Item is Hashable as well as Identifiable (that's the part I think you wouldn't like), so that the OutlineViewDataSource can hold onto a dictionary of [ID : Int] (the ID of each Data.Element currently displayed, along with its hashValue), then changing OutlineViewUpdater so that it takes that dictionary into account, and is able to determine which cells need a reload based on the hashValue of the individual items.

In that way, cells would reload when data updates occur and the the Data.Element for a row has any changes from the previous version of the row (which would have a different hashValue). Is that way too hacky? It doesn't necessarily fix the version of the issue I show in the recording above, where all rows need an update based on a global value rather than on Data.Element values, but the same effect could be achieved if each Data.Element has a color value that is accounted for in its hashValue.

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.

2 participants