Giter Club home page Giter Club logo

composedui's People

Contributors

charleshs avatar josephduffy avatar shaps80 avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar

composedui's Issues

Section removals cause wrong calls to `CollectionCellElement`'s `didDisappear`

Describe the bug

When a section is deleted the didDisappear call on CollectionElementsProvider is either not made or is made on the wrong CollectionElementsProvider.

To Reproduce

Delete a section.

Expected behavior

Correct calls to didDisappear should be made/should not crash.

Environment

  • OS Version: Any iOS
  • Library Version: 1.05
  • Device: Any

Additional context

CollectionCoordinator.collectionView(_:didEndDisplaying:forItemAt:) uses the state of the hierarchy after the more recent updates have been applied, meaning the removed cell/section is no longer in the SectionProviderMapping or the local cachedProviders.

For example deleting section 1 with a hierarchy of:

  • section 0
    • 0-0
    • 0-1
    • 0-2
  • section 1
    • 1-0
    • 1-1
  • section 2

Then remove section 1:

  • Crashes when the cell is cast to the wrong type
    • This happens when section 1 and have different cell types
  • Calls the wrong CollectionElementsProvider
    • This happens when section 1 and have the same cell types
    • This would call didDisappear on section 2 (even though no cells were removed and it's already empty)

Or when section 2 is removed:

  • Makes no call at all
    • This happens because of guard indexPath.section < sectionProvider.numberOfSections else { return }

I don't have time to look at this right now and I'm not as familiar with the SectionProviderMapping as other parts, can you try and fix this @shaps80?

A suggested solution is to cache the cells and their sections (e.g. cellSectionMap: [UICollectionViewCell: Section]) but I'd like to know if there's a alternative approach since that might lead to more issues down the line?

Crash when invalidating then inserting/deleting

Describe the bug

A crash can occur when using invalidateAll followed by an update (I've only triggered this with an insert but others could work, I suspect delete too).

To Reproduce

Call invalidateAll, followed by an insert/delete. I think the invalidateAll call needs to trigger a layout.

Expected behavior

It should not crash.

Environment

  • OS Version: iOS 13.7
  • Library Version: master (8ee5b11, 1.0.3 + some fixes)
  • Device: iPod Touch

Additional context

I think the issue comes down to how the API for the delegate and UICollectionView itself differ. My understanding is that calling reloadData (which is done on an invalidate) does not guarantee that the changes have been fully applied after reloadData occurs, e.g. a layout pass might be required.

When looking at the docs for performBatchUpdates it states:

If the collection view's layout is not up to date before you call this method, a reload may occur. To avoid problems, you should update your data model inside the updates block or ensure the layout is updated before you call performBatchUpdates(_:completion:).

This would be ok if we were updating the data model inside the updates block, but updates are surrounded by willBeginUpdating and didEndUpdating. didEndUpdating triggers the call to performBatchUpdates but by this point the models aren't in-sync with what the collection view thinks is true.

So I think when we call reloadData and then performBatchUpdates before the layout has occurred the collection view will try to update straight away. This starts with numberOfSections which will return the new value, but when the collection view then requires for a cell it will eventually hit elementsProvider(for:) which will crash because the cachedProviders has not been updated yet because the prepareSections call occurs inside the performBatchUpdates.

I tried calling layoutIfNeeded before performBatchUpdates. This essentially triggers the same crash since I think that's what performBatchUpdates is doing internally

I also tried calling collectionView.layoutIfNeeded() straight after collectionView.reloadData() which fixes the crash but seems to cause a visual bug so I'm not sure if this can be classed as a fix or not.

Ultimately the fix for me was to remove invalidateAll and do diffing, but since most of the built-in types use invalidateAll this isn't really a fix either.

Ultimately I think the fix is to update the delegate in such a way that the model updates can occur inside the performBatchUpdates call, but this would need some more thought an a major version bump.

Crash when adding a section before view appears

Describe the bug

When adding a section to a ComposedSectionProvider in a viewDidLoad after the CollectionCoordinator has been set as the delegate but before the view has appeared the coordinator will fatalError in elementsProvider(for:) with error Fatal error: No UI configuration available for section 2.

To Reproduce

  1. Checkout https://github.com/composed-swift/Composed-Demo/tree/ComposedSectionProvider-crash-example
  2. Uncomment https://github.com/composed-swift/Composed-Demo/blob/ComposedSectionProvider-crash-example/Composed-Demo/Collection/PeopleCollectionViewController.swift#L20

Expected behavior

Should not crash

Environment

  • OS Version: iOS 13.5, iOS 14 beta 7
  • Library Version: 1.0.2
  • Device: iPhone 8 Simulator

Additional context

The crash is triggered by collectionView.performBatchUpdates, but the closure is never executed so prepareSections never gets a chance to be called.

Function Builder API

Adding FunctionBuilder support

Introduction

I'd like to add support for the new function builder APIs.

Motivation

The current implementation requires the creation of a class with the inclusion of at least 1 method. In addition, it is required to create at least 1 cell class (and associated XIB?).

This doesn't sound like a lot but in practice it can still quickly become cumbersome when building reusable code.

Most often its necessary to define your own generic types on top of Composed's options:

final class PeopleSection: ArraySection<Person>, CollectionSectionProvider {

    func section(with traitCollection: UITraitCollection) -> CollectionSection {
        let cell = CollectionCellElement(section: self, dequeueMethod: .nib(PersonCell.self)) { cell, index, section in
            let element = section.element(at: index)
            cell.prepare(with: element)
        }
        return CollectionSection(section: self, cell: cell)
    }

}

Even in this rudimentary example, that's still a fair amount of code. What's important to capture here is:

  • Our data will be backed by an Array
  • Our data will be presented in a UICollectionView
  • Our data will be represented by a PersonCell

Proposed solution

Add a FunctionBuilder style DSL to simplify usage for an API consumer as well reduce the code required to achieve the same result.

The following represents an 'idea' rather than a concrete example. There are still a few areas that need to be worked out before this is possible, as such the following code is for demonstration purposes only and is subject to change.


Section(people) { index in
    Cell(.fromNib(PersonCell.self) { cell in
        cell.prepare(people[index])
    }
}

There are a few added benefits to this approach:

  • We 'inject' the data (in this case an Array)
  • Our closures are simpler, our section returns an index (to the element) and our cell returns an instance of the cell to configure
  • We no longer need to subclass in order to specify our backing data

Presentation

One thing to note, Cell needs to be inferred so the above wouldn't compile until it's inserted into a view. A nice solution to this might look like the following:

struct Dashboard: CollectionView {
    var content: Content  {
        Section(friends) { ... }
        Section(family) { ... }
    }

}

Using this approach we can see:

  • The use of a struct removes reference semantics and the potential for retain cycles
  • We can infer that a UICollectionView is intended for the presentation
  • We can also now infer that a UICollectionViewCell is what we're expecting for our Cell configuration
  • We can easily compose multiple sections

Headers & Footers

However another added benefit to using the view to infer our Section type is that we can also now determine what 'features' our Section might have.

For example, we know that our UICollectionView supports header/footer elements:

struct Dashboard: CollectionView {
    var header: Header {
        Header(.fromNib(PeopleHeader.self)) { header in
            header.titleLabel.text = "People"
        }
    }

    // and to add the header to the section...
   
    var content: Content  {
        Section(header: header) { ... }
    }
}

We could even go further, providing a convenience header type:

Section(header: Text("People"))

Static Views

One final advantage over this API is that it makes it trivial to build both dynamic as well as static Section's.

Section {
    Cell(.fromNib(PersonNameCell.self) { cell in
        cell.prepare(person.name)
    }

    Cell(.fromNib(PersonAgeCell.self) { cell in
        cell.prepare(person.age)
    }
}

It may even be possible to implement ForEach such that mixing static and dynamic content would become possible.


Existing Sections

In the current implementation we have a few Section types that define the backing data that will be used for that section.

  • SingleElementSection
  • ArraySection
  • ManagedSection

As well, we have two SectionProvider types that help us compose sections:

  • ComposedSectionProvider
  • SegmentedSectionProvider

Using the proposed implementation, we can see all of these become completely unnecessary.

SingleElementSection

A SingleElementSection can be replaced by simply defining a static section:

Section {
    Cell(.fromNib(PersonNameCell.self) { cell in
        cell.prepare(person.name)
    }
}

ArraySection & ManagedSection

Both can be defined by simply providing the relevant data to the Section

ComposedSectionProvider

Simply including multiple sections removes the need for a specific type as well.

SegmentedSectionProvider

That leaves us with just one final type. The purpose of this type is to hold onto 1 or more child Section's while keeping only 1 active at a time.

Well, this is easily achieved with the above implementation simply with the use of conditional statements.

Section(...)

if isVisible {
    Section(...)
}

Section(...)

Conclusion

It's now apparent that this implementation should remove the need for an API consumer (at least) to ever have any knowledge of multiple Section types. Instead focus on providing the data (or not) for a Section and specifying which Cell they want to be used for representing the element at a specified index.

As for composition, using Swift knowledge you already have, you should easily be able to build composable Section's using nothing more than conditional statements.

The use of FunctionBuilder APIs also removes the additional syntax of brackets and comma's that would otherwise (and currently) litter your code.

Behaviours

ComposedUI also extends your Section through the use of protocol conformance to provide behaviours like editing, selection, drag and drop and more.

One potential solution here would be the use of a Modifier-style API as seen in SwiftUI:

struct PeopleSection: CollectionSection {
    var header: Header {
        Text("People")
            .attributes(...)
            .padding(.horizontal, 20)
    }
    
    var content: Content {
        Section(header: header, people) { index in
            Cell(.fromNib(PersonCell.self)) { ... }
                .onSelection { ... }
                .onDrag { ... }
                .onAppear { ... }
                .contextMenu { ... }
        }
    }
}

There are a few obvious benefits to this approach:

  • Call order is not important
  • Behaviour is still additive (not implementing the modifier, implicitly disables it)
  • Its a familiar API (if you've used SwiftUI) and its extremely easy to discover
  • In future versions Composed will likely support SwiftUI and this API will 'feel' a lot more familiar and consistent.

Final Thoughts

This approach is not only simpler and cleaner, its also much more user friendly and discoverable. It removes a few key pain points for users as well:

  • No need to subclass, or even use a class
  • No need to conform to other protocols (that you always need to 'know' about)
  • Data, indexing and presentation are truly separated, only coming together at the point where you embed them in a view
  • You 'describe' the sections more naturally

One last thing...

There are 2 other areas that still need to be thought through.

  1. Layout
  2. Environment

Layout, can likely be solved by initialising the view with a layout (if required). The bigger question is how to tie individual section layouts to the outer view layout. However we've solved it already, so I'm sure this can be worked through.

Environment variables are passed to functions or closures currently. These could replaced by something more like @Environment available in SwiftUI where Composed ensures the variables are set automatically for you at the right moment. I'm not sure atm if this is possible or something the compiler does for free in SwiftUI alone. I will need to investigate this further. Alternatively, the functions/closures could continue to provide these values as required.

Considerations

The current approach to handle section updates and ensure they get propagated up to the coordinator is to use the delegate pattern. In order to support this 1 of 2 things should be considered.

  1. Is this API more of a sugar-candy DSL that just makes it simpler to define your sections, or
  2. Is this a complete re-architecture of how Composed is implemented

My current instincts are to consider the usage of Combine and the new Swift diffing APIs, however this would mean dropping support for iOS <13. I'm not adverse to this, but it should be considered carefully.

Source compatibility

This is likely to cause breaking API changes and require significant renaming in both ComposedUI and Composed. As such its planned for 2.0.

Alternatives considered

N/A

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    ๐Ÿ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. ๐Ÿ“Š๐Ÿ“ˆ๐ŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google โค๏ธ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.