search

thervh70 / ContextProject_RDD

1 stars 0 forks source link

84 donotwatch frontend #108

Closed thervh70 closed 8 years ago

thervh70 commented 8 years ago

Will close #84

Front end for the DoNotWatchOptions. Please confirm functionality and check code.

coveralls commented 8 years ago

Coverage Status

Coverage remained the same at 87.559% when pulling f02974251e56e4ba98abf04a07176bc28f78c9c6 on 84-donotwatch-frontend into 850fb2585b9d3286f92ef6d11bd1a6a14ba3c243 on dev.

mpsijm commented 8 years ago

Please document this more clearly, because I don't get it so users probably won't get it either... when I enable the checkbox, the watching of hover/comments/etc will be disabled? In either case, when I check or uncheck the "hover" checkbox, events 202 and 203 (mouse-enter/-leave) are still logged...

thervh70 commented 8 years ago

This feature is not working indeed. However that is not part of my task. I will discuss with @Exclaminator on how to fix this. I will decide on how to present the options to the user (texts on the options page) as soon as the logic behind it is working.

mpsijm commented 8 years ago

Fair enough, since the front-end works, I approve the functionality of this PR. Please add a TODO on the options page in that case, then this can be merged. :) EDIT: Done, can be merged :)

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.8%) to 88.361% when pulling 2d90b2715e1c345026a34748bca3c17fde1de788 on 84-donotwatch-frontend into 850fb2585b9d3286f92ef6d11bd1a6a14ba3c243 on dev.

MathiasMeuleman commented 8 years ago

In my opinion that is the wrong way around. You should first decide how the user should be able to view and change the settings and after that you adapt the back-end to fit these elements. This means that the front-end should still be changed in this PR, preferably in such a way that either the negations completely disappear, or else in such a way that it is made very very clear it is a negation ('Feature Selection' does not indicate a negation)

mdingjan commented 8 years ago

Side note: once this PR is merged, I will change the changes so that they are compatible with my refactorings. ;)

thervh70 commented 8 years ago

Done! This can be merged now.

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.9%) to 88.441% when pulling ff9d4dc1125164b946907db817e84c585eaac78d on 84-donotwatch-frontend into 850fb2585b9d3286f92ef6d11bd1a6a14ba3c243 on dev.

mdingjan commented 8 years ago

I approve the changes for my feedback. When @MathiasMeuleman has approved the changes for his feedback as well, this PR can be merged.

mpsijm commented 8 years ago

Can be merged now in my opinion as well, please check my last comment before merging :)

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.9%) to 88.441% when pulling c31d63d1a50247c9ba5304e6d91407f60ba4d835 on 84-donotwatch-frontend into 850fb2585b9d3286f92ef6d11bd1a6a14ba3c243 on dev.

MathiasMeuleman commented 8 years ago

Thank you so very much. Will merge :)