No offence taken at all 
If starting the node again - then that might be a neater approach - but it would make accessing the properties more work and Paul's driving force was to keep things as simple as possible - and of course I'm the greatest proponent of that philosophy also - hence my handle
First post with previous linked thread explains why I'm doing this
I thought it would be a good learning exercise for me as well - I've done some minor node PRs in the past. This has been a good learning exercise for me
1 Like
I think the buttons look more natural at the top, that's where I think most similar functions are placed, do you agree?
Yes, looking back, that may have been a better approach, but I wrote simpletime almost 3 years ago (and probably my first stab at writing a contrib-node...), and it's really too late to start making breaking changes of that magnitude now.
I realise that I could also add msg.simpletime.hour etc, as well as the existing msg's, but that's adding even more pollution into the mix.
...and for me. I'm going to have a good look at how you have done this, and also learn from it.
2 Likes
Ahha, I see. In which case Simon's approach is perfectly reasonable & the right choice.
And glad no offence was taken (critiquing can go bad no matter how well intentioned)
3 Likes
Entriley up to you - they are your feature request
Choice 2A below or previous Choice 2?
Well....could be an exercise in how not to I REALLY have little grasp of HTML/JS but I have good Google/copy/paste skills
3 Likes
I took it quite well considering that we should have boarded a plane 1 hour ago for a spa holiday in the med.. 
2A - the version with the buttons on the top looks better IMO.
1 Like
This will be final layout unless I hear anything
Just off to test it out on another install before doing the PR
1 Like
@Paul-Reed Due to mix up at my end - can you delete your existing tickboxes branch and re-create it and I'll re-fork and update and then do the PR back to you
Sorry for extra admin!
I think that the 'backwards compatibility' issue has been resurrected.
In the flow below, I have an existing v2.9.1 simpletime flow, and have then updated simpletime to the 'tickboxes' branch, and restarted node-RED.
You'll notice that the debug shows no data, yet the now updated node shows all of the options ticked.
Even 'deselecting' and then 'selecting all' does not prompt a deploy request, or change the blank output msg.
Adding a fresh simpletime node from the sidebar, does however work as expected.
Can you so a Ctrl-Shift-I to bring up the console and see if any errors are popping up?
I'm thinking that maybe I need to something in simpletime.js to check if the properties don't exist (for whatever reason) and default them to true
But I'd really like to fix it at source in simpletime.html
No errors in the browser console log, and nothing in the node-RED log either.
1 Like
mm this is interesting and might help out the update issues
On my testing (not developing) machine - PiZero
I npm removed tickboxes branch - I deleted whole simpletime node folder - I removed it using pallette (as it was still showing) I restarted NR and Hard reset/cleared cache - so as close to a clean new install as I could get
I then added simpletime via pallete and added it to flow and got this
Ok - this is my theory 
The simpletime.js needs one of the JS things like this in the core debug node
function DebugNode(n) {
var hasEditExpression = (n.targetType === "jsonata");
var editExpression = hasEditExpression ? n.complete : null;
RED.nodes.createNode(this,n);
this.name = n.name;
this.complete = hasEditExpression ? null : (n.complete||"payload").toString();
if (this.complete === "false") { this.complete = "payload"; }
this.console = ""+(n.console || false);
this.tostatus = n.tostatus || false;
this.statusType = n.statusType || "auto";
this.statusVal = n.statusVal || this.complete;
this.tosidebar = n.tosidebar;
if (this.tosidebar === undefined) { this.tosidebar = true; }
this.active = (n.active === null || typeof n.active === "undefined") || n.active;
to make each property = true if it doesn't exist when processing the message
But I'm totally confused as to which one I should use for this (my brain hurts with JS undefinded/null/==/===/|| clever one line stuff)
particularly the use of ||
My "Monkey see- Monkey do" approach to coding has failed me at this point
I've just rolled my installation to v2.9.1 (the npm version before any of the recent changes) and set up a series of flows with simpletime, rebooted, and then updated simpletime to v2.9.3 ( the version before the last update).
The flows worked fine, no problems, and it handled the backwards compatibility fine.
However, something in your last PR seems to have changed things, as per my post above and it now introduces a breaking change.
Right - I think I FINALLY have a repeatable test sequence.
I clear my test folder - do an alternate install into it - run it - add simpletime from palette which gets me 2.9.1 and works
I then install my github tickboxes fork branch from command line - restart alternate NR - F5 refresh broswer and I see the same fault as you
So hopefully, I can try fixes (I reckon I need stuff in simpletime.js) and we'll see what happens
(I'm planning on trying to fix some of the properties to prove that I'm on the right track at first to save me being fooled (again) that I've sorted things
1 Like
PROGRESS 
I altered two properties in simpletime.js and both methods seemed to work for mydate and myymd
Going to use the this.ymd approach for all the lines now - update -redo my test install from scratch and see if The Force is with me
[edit]The Force has told me go away and try again 
Before I re-created my test install I tried out just upgrading - It seems it works on an existing node that was there before I "upgrade" but doesn't work properly with checkbox select/unselect if I create a 2nd inject-simpletime-debug sequence [/edit]
2 Likes
OK - I've added node.wearn debuging to simpletime.js and code notices that config object doesn't have any checkboxes on a previous installed node, so it adds them into this object going forward
and if I add another simpletime node - it shows that the this object has got the checkboxes so it just passes them through unchanged
So I finally 
think I'm there - just going to comment the node.warns out - do a final test and then do the PR again
@Paul-Reed There's a fiver in the post for you (or paypal transfer ) if this doesn't pass your testing !
I've issued a PR - I've called this one v.2.9.7
... of course - it has thrown a slight spanner into works but I've pressed go anyway!
if the PR causes issue for you - just delete/recreate branch and I'll re-fork again like last time
Not at all!
I've merged your PR into the compat branch, and just updated my original NPM version with it.
You'll be pleased to hear that I won't be getting your fiver!!
If you are happy with things, I'll publish it to NPM later tonight as v2.10.0
Nice work Simon
1 Like
