Cosmo / TinyConsole

📱💬🚦 TinyConsole is a micro-console that can help you log and display information inside an iOS application, where having a connection to a development computer is not possible.
MIT License
1.95k stars 89 forks source link

Add custom actions for 3 finger tap #23

Closed popodidi closed 7 years ago

popodidi commented 7 years ago

I added a TinyConsoleThreeTapDelegate and implemented it in AppDelegate to customize the action sheet of 3 finger tap.

mRs- commented 7 years ago

It seems like an anti pattern for me. I think it would be better to give this configuration via dependency injection or some kind of builder pattern. It seems wrong to hold a static delegate for this kind of task.

It must not be highly dynamic imho, but maybe cosmo can give us a better explanation about where this thing might be heading.

Configuration stuff is for me not vialable via delegating, it smells a little bit.

Am 25.12.2016 um 04:54 schrieb Hao notifications@github.com:

@popodidi commented on this pull request.

In TinyConsole/TinyConsole.swift:

@@ -88,6 +88,13 @@ open class TinyConsole { public static func addMarker() { TinyConsole.print("-----------", color: UIColor.red) }

  • // MARK: - Actions
  • /**
  • 3 Finger Tap action delegate
  • */
  • public weak static var threeTapDelegate: TinyConsoleThreeTapDelegate? I thought it might be simpler and more consistent with other APIs by setting TinyConsole.threeTapDelegate rather than TinyConsole.shared.threeTapDelegate. Since I found you are deprecating old functions and replace them with static ones. Would it be better that TinyConsole is never initiated and APIs are always static func / var ?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

Cosmo commented 7 years ago

I think it would be best, if TinyConsole could be as plain as possible when needed (no default gestures or sheets). Maybe we should move everything (gestures, sheets) out -- as a default implementation hook that can be enabled or disabled if a developer wants to add custom gestures.

To be honest, the gestures and actions that are pre-configured, are my personal preferences as hardcoded defaults. But I didn't give to much thought to it, as TinyConsole provided exactly what I needed ;)

mRs- commented 7 years ago

But who is in charge of this? TinyConsole or TinyConsoleController?

popodidi commented 7 years ago

I would probably choose TinyConsole. I feel that TinyConsole is more like an interface to call functions than a class to initiate. In this point of view, even TinyConsoleController(rootViewController:) is a bit confusing to me since I don't want to be calling function other than TinyConsole...... I would be happy to have some kind of TinyConsole.createViewController(_:) to init root view controller. Back to our gestures and actions, maybe I can try to define a var consoleViewController: TinyConsoleViewController? as you did for textView, and reference it in TinyConsoleViewController.viewDidLoad ? In this approach, developers can configure by TinyConsole.shared.consoleViewController.view.addGesture....... And with some sort of func useDefaultGestureConfiguration(), we could do default configurations. It's a bit verbose though. Referencing TinyConsole.shared.consoleView to TinyConsoleViewController.view should be enough to configure.

mRs- commented 7 years ago

That seems a bit of and the API is not easy to discover. The approach to configure it first and then call something like TinyConsole.createViewController would be the best option.

IMHO TinyConsole should be the only interface that is really public and can be interacted with. I think it would be less confusing to implement this at all.

Currently you have to create a TinyConsoleController (a ViewController) to implement your rootViewController.

The better way would to initiate TinyConsole the first time and configure it for you to work with. To easily give an example there should be some standard actions to implement to give the developers a way to create new actions.

TinyConsole.shared.consoleViewController.view.addGesture...... is too much and you have to dive very deep to implement new gestures for this and it's not easy accesible.

popodidi commented 7 years ago

How about this implementation? I have deprecated the public init of TinyConsoleController and put all the APIs as static functions of TinyConsole, where I call shared.consoleController... to implement functionalities. Ideally, the var consoleController of TinyConsole.shared should never be changed as we do gesture configurations or set(rootViewController:). In this way, the gesture configurations could hold even the rootViewController is changed.

popodidi commented 7 years ago

I have updated the README.md file. Please kindly check if this is a feasible solution to configureTinyConsoleController and to customize gesture recognizers.

mRs- commented 7 years ago

I like the implementation so far. But it's breaking the current implementation of the Public API. @Cosmo has the last call about this

Cosmo commented 7 years ago

Ok, let's do this.