exyte / Macaw

Powerful and easy-to-use vector graphics Swift library with SVG support
MIT License
6.01k stars 553 forks source link

Fixed `onTap(..)` handler - unable to add second tap handler #715

Closed devpolant closed 4 years ago

devpolant commented 4 years ago

Updates

In this pull request I'm going to fix the logic of registering new tap handlers.

Previous implementation of this method is completely wrong, because copy-on-write behaviour was not considered.

New handler was appended to copy in this line - handlers.append(handler). tapHandlers dictionary was not changes, so as a user:

This is old implementation:

    @discardableResult public func onTap(tapCount: Int = 1, f: @escaping (TapEvent) -> Void) -> Disposable {
        let handler = ChangeHandler<TapEvent>(f)
        if var handlers = tapHandlers[tapCount] { // array copy is created
            handlers.append(handler) // copy is mutated here, so changes not applied to `self.tapHandlers`
        } else {
            tapHandlers[tapCount] = [handler]
        }

        return Disposable { [weak self, unowned handler]  in
            guard let index = self?.tapHandlers[tapCount]?.firstIndex(of: handler) else {
                return
            }

            self?.tapHandlers[tapCount]?.remove(at: index)
        }
    }

Issue is fixed in new implementation.

ystrot commented 4 years ago

Thank you for the fix! Merged it.