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

Consider removing ViewModelRendering #130

Open
krris opened this issue May 17, 2016 · 9 comments
Open

Consider removing ViewModelRendering #130

krris opened this issue May 17, 2016 · 9 comments
Assignees

Comments

@krris
Copy link
Contributor

krris commented May 17, 2016

Consider removing ViewModelRendering.

@ochococo
Copy link
Contributor

@krris What do you mean?

@krris krris changed the title ViewModelRendering protocol is not used Consider removing ViewModelRendering May 30, 2016
@krris
Copy link
Contributor Author

krris commented May 30, 2016

Currently ViewModelRendering is only used by two widgets: WatchWidgetView and ImageWidgetView. All other widgets don't use it. We should think if this protocol is still needed. It's worth noting that in README it's said:

View: a view implementing ViewModelRendering protocol that displays the information.

but only two of our widgets follow the guidelines.

@ochococo
Copy link
Contributor

ochococo commented May 30, 2016

So there are two Widgets using it.
Why the rest doesn't? I don't know. Ask the authors.

This protocol is here to stay, not sure about those Widgets (they come and go as you know).

@ochococo
Copy link
Contributor

We could talk about it and maybe introduce another method configureWithViewModel: and mark the old ones as deprecated?

@ochococo
Copy link
Contributor

ochococo commented Jun 1, 2016

But with this response latency @krris it will be really hard to discuss 😀

@ochococo
Copy link
Contributor

ochococo commented Jun 1, 2016

WDYT @michallaskowski ?

@krris
Copy link
Contributor Author

krris commented Jun 1, 2016

Sorry for late response but I missed notifications. So let's leave as it is. Also I would prefer to do as you say:

introduce another method configureWithViewModel: and mark the old ones as deprecated

Maybe we should refactor other widgets a little to follow the guidelines?

@michallaskowski
Copy link
Contributor

It is not the problem with widgets not using it, because Core is not using it. Then there is no point in having this protocol.

I am in for removing it, since it has no effect. Because widgets are responsible for creating views, we should not force MVVM on them. Current protocol for 'WidgetControlling' is enough for me.

@ochococo
Copy link
Contributor

ochococo commented Jun 1, 2016

@krris We can refactor but one thing should be clear: there is noting like "ErrorWidget" or "ErrorMode" - not every Widget should have "erroneous mode" and something like blank error view on Clock widget is a big mistake.

@michallaskowski I would like to "guide" the developer not to enforce (as there is no constraint, just a convention).

@ochococo ochococo self-assigned this Jun 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants