-
Notifications
You must be signed in to change notification settings - Fork 2
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
Redux.NET: Change how the delete button visibility is controlled #4
Comments
I disagree, I think whether or how the delete buttons appear is a view concern. |
I was torn when I did the initial implementation and I felt the same as you. However, after some more thought I'm sure it's wrong as it is. At the moment there's a bit of business logic that we can't test because it's in the view. I think it might be best to create a component for the to-do item. Then dispatching an action to control the button behaviour seems right. I need to look into how components interact with the store though. Sent from my Google Nexus 6 using FastHub |
Ah, but you can test it; using test stack.white or something similar. Anyway, while i'm for giving the business logic as much test coverage as possible, i'm not sure if visual effects in the view count as business logic... Put it this way, the idea behind separation of view and business logic is that ideally we develop the business logic seperately, and some UX expert goes away and develops the UI, complete with whatever fancy visual effects they want. If we start dragging the logic of visual elements like that into the viewmodel, (or the data store / reducer) then we are essentially prescribing fine details of the view and getting in the way of the UX designers job. Granted there is a line to be drawn at where this ends, obviously we need to prescribe some elements, and I don't have a clear rule in mind for where we draw the view / business logic distinction, but I think there has to be one. Lets discuss this tomorrow, it's time for game of thrones. |
The visibility of the delete button should be handled in the business logic rather than in the view as it is now.
The text was updated successfully, but these errors were encountered: