search

thervh70 / ContextProject_RDD

1 stars 0 forks source link

136 Default Options Button #146

Closed mdingjan closed 8 years ago

mdingjan commented 8 years ago

Will close #136

The restore default options button has been added.

The way I've implemented it is quite simple; an object is created based on the frontend defaults on initialisation. This object is used for updating the chrome storage and the checkboxes.

Please review this PR by rebuilding the extension and playing around with the new button. Tell me whether you like the button and whether you can detect any bugs.

Please note that in another PR all default options will be moved to the backend.

coveralls commented 8 years ago

Coverage Status

Coverage remained the same at 93.593% when pulling 619ee1de8172bf03199b2ec3ba6b40c4952e61c6 on 136-default-settings-button into 494bce3c4f356d24a6dca9b90033169e330e3961 on dev.

coveralls commented 8 years ago

Coverage Status

Coverage remained the same at 93.593% when pulling b65e179553e76748c468cd15799c4629637a2291 on 136-default-settings-button into 494bce3c4f356d24a6dca9b90033169e330e3961 on dev.

mpsijm commented 8 years ago

How about a margin below the button? I'd also change the alignment of the boxes but that should be another issue :P The placement of the button relative to the other settings looks to me though :)

https://i.gyazo.com/1b0d45f7d1a0122030f5b9a43f344365.png

MathiasMeuleman commented 8 years ago

Please have a look at the comments @mpsijm and me posted, after that I think this is done

coveralls commented 8 years ago

Coverage Status

Coverage remained the same at 93.593% when pulling 2e4bb50ba065359697890adec0b8c17df0966d89 on 136-default-settings-button into 494bce3c4f356d24a6dca9b90033169e330e3961 on dev.

mdingjan commented 8 years ago

I've resolved the feedback; you can find the changes in my two latest commits for this PR. Thanks! :)

coveralls commented 8 years ago

Coverage Status

Coverage remained the same at 93.593% when pulling 8ce5fe21a2062e2f77c0e288c0bf6d4869319093 on 136-default-settings-button into 494bce3c4f356d24a6dca9b90033169e330e3961 on dev.

mdingjan commented 8 years ago

@thervh70 @mpsijm Please check the latest commit. :)

coveralls commented 8 years ago

Coverage Status

Coverage remained the same at 93.593% when pulling 0b3651bbcd9b1327593d2152aa4da8539592c6ef on 136-default-settings-button into 494bce3c4f356d24a6dca9b90033169e330e3961 on dev.

mpsijm commented 8 years ago

Thanks for processing our feedback, Mitchell! :) I think Robin can merge this if he agrees with your changes as well, since the majority voted for not having a toaster :)

thervh70 commented 8 years ago

Fair enough. Looks good Mitchell. Will merge!