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

How to use ComposedSectionProvider with Sections that update? #2

Closed
JosephDuffy opened this issue Jul 6, 2020 · 4 comments
Closed

Comments

@JosephDuffy
Copy link
Member

When calling ComposedSectionProvider.append(_:) with a Section the updateDelegate is not set (and ComposedSectionProvider does not conform to SectionUpdateDelegate). This causes any updates propagated by the section to not be honoured.

It looks like these updates should be propagated up, performing mapping as required. Does SectionProviderUpdateDelegate need to inherit SectionUpdateDelegate to ensure these updates are not lost?

@shaps80
Copy link
Collaborator

shaps80 commented Jul 6, 2020

Hmm the demo app even adds removes sections without issue. Which wouldn’t be possible if the updateDelegate wasn’t set.

@JosephDuffy
Copy link
Member Author

JosephDuffy commented Jul 6, 2020

Hmm the demo app even adds removes sections without issue. Which wouldn’t be possible if the updateDelegate wasn’t set.

Looking at the demo the delegate is set to an instance of SectionProviderMapping but I can't find where that's done.

I think I got confused about what it was embedded in. For example I have:

  • ComposedSectionProvider
    • ComposedSectionProvider
      • ComposedSectionProvider
        • SingleElementSection
        • SegmentedSectionProvider
          • Section

Changes made to the Section are not propagated and the delegate is nil, maybe this is actually an issue with SegmentedSectionProvider.

Edit

Making PeopleComposedSectionProvider a SegmentedSectionProvider and updating the insert to:

    var newIndex = 0

    func append() {
        append(peopleSection())
        self.currentIndex = newIndex
        newIndex += 1
    }

causes new inserts to not be shown after inserting a second section. A breakpoint at https://github.com/composed-swift/Composed-Demo/blob/master/Composed-Demo/Sections/PeopleSection+Editing.swift#L37 shows that updateDelegate is nil.

I can't figure out exactly how it should work but here's a test that I think should pass but doesn't:

import Quick
import Nimble

@testable import Composed

final class SegmentedSectionProvider_Spec: QuickSpec {

    override func spec() {
        describe("SegmentedSectionProvider") {
            var global: SegmentedSectionProvider!
            var mapping: SectionProviderMapping!
            var child1: ComposedSectionProvider!
            var child2: ArraySection<String>!

            beforeEach {
                global = SegmentedSectionProvider()
                mapping = SectionProviderMapping(provider: global)

                child1 = ComposedSectionProvider()
                    let child1a = ArraySection<String>()
                    let child1b = ArraySection<String>()
                child2 = ArraySection<String>()

                global.append(child1)
                global.append(child2)

                child1.append(child1a)
                child1.append(child1b)
            }

            context("after changing the `currentIndex") {
                beforeEach {
                    global.currentIndex = 1
                }

                it("should unset the delegate of the previous child") {
                    expect(child1.updateDelegate).to(beNil())
                }

                it("should set the delegate of the current child") {
                    expect(child2.updateDelegate).toNot(beNil()) // Fails
                }
            }

            context("without changing the `currentIndex`") {
                it("should set the delegate") {
                    expect(child1.updateDelegate).toNot(beNil()) // Fails
                }

                it("should not set the delegate of children") {
                    expect(child2.updateDelegate).to(beNil())
                }

            }
        }
    }

}

JosephDuffy added a commit to JosephDuffy/Composed-1 that referenced this issue Jul 6, 2020
Basic `Section`s were not propagating updates if added via a call to `invalidateAll`. This is to address composed-swift#2.

The issue can be recreated by using a `SegmentedSectionProvider` when switching to a segment that contains a `Section` (`SectionProvider` works I _think_)

This commit alone is likely not enough to fix this issue; `updateDelegate` should probably be set to `nil` for removed `Section`s, and should probably be set in more places.
@shaps80
Copy link
Collaborator

shaps80 commented Aug 24, 2020

I will review this soon and get back to you.

@shaps80
Copy link
Collaborator

shaps80 commented Aug 25, 2020

This has been resolved and included in release v1.0.3

@shaps80 shaps80 closed this as completed Aug 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants