feat(#41): Camel preferences

[joshiraez]

This together with #191 completes #41

Notes:

• Right now values are being saved to local storage and little else. Would like to get your input in if any components already exist to put the wiring on them.
• Reset button. Should I add it? Home preferences didn't have a Reset button before and it has it, same for Jolokla settings. I guess it's a yes but wanted to confirm.
• Hint icons. I saw we dropped this from other preferences pages. Should I file a separate issue to implement them on all preferences pages?
• Overuse of arrow functions on the component: trying to reduce the amount of duplicated code for type checking, I had to use arrow functions because I lose the "this" when passing them as parameters. Is not too much boilerplate... until prettier comes up and puts everything in different lines :(. If
  • I remember that the correct pattern to avoid that was to store this in a const variable in the class, but typescript seemed to not like it.

I think that's all I will otherwise put another comment tomorrow

[tadayosi]

And now I realise that this is only half of what #41 should implement. We also need to implement the consumer side of the preferences, since there should be consumers in the Camel plugin that uses these parameters to tune their logics.

So, at least, the camelPreferencesService should provide each method for a corresponding preferences parameter that is supposed to be used from some Camel view elsewhere. In the old Hawtio, these methods were implemented here:
https://github.com/hawtio/hawtio-integration/blob/main/plugins/camel/ts/camelHelpers.ts

@phantomjinx @mmelko Are there any places in the Camel views you've implemented where the preferences parameters should be referred to but haven't been yet since the prefs hadn't been implemented yet?

[tadayosi]

• Right now values are being saved to local storage and little else. Would like to get your input in if any components already exist to put the wiring on them.

Yes, that's the latter half of the task. But it can be broad and some may be even yet to be implemented, we can work on it in a separate pull req. In this pull req, I think it's ok so long as the camelPreferencesService provides the getter methods for all parameters.

• Reset button. Should I add it? Home preferences didn't have a Reset button before and it has it, same for Jolokla settings. I guess it's a yes but wanted to confirm.

No, you don't need it. As I explained before, we only need two reset buttons in the entire Hawtio console. One is to reset every setting except connections, and the other is to reset connections specifically. That's why I said localStorage.clear() is enough for the former reset button. Probably you're still misunderstanding something.

Home preferences didn't have a Reset button before and it has it, same for Jolokla settings.

I don't see what it means. The old Hawtio has been having the two buttons and nothing changes at this point. One thing to note is that the old General and Reset are refactored into single Home prefs, and Jolokia and Connect are refactored into Connect, because the old prefs were rather cluttered with prefs with sparse options.

• Hint icons. I saw we dropped this from other preferences pages. Should I file a separate issue to implement them on all preferences pages?

Good point. Yes, please file another issue and let's tackle it separately. I agree it's good idea to restore the hints.

• Overuse of arrow functions on the component: trying to reduce the amount of duplicated code for type checking, I had to use arrow functions because I lose the "this" when passing them as parameters. Is not too much boilerplate... until prettier comes up and puts everything in different lines :(. If I remember that the correct pattern to avoid that was to store this in a const variable in the class, but typescript seemed to not like it.

Possibly my suggestion above to consolidate parameters into a single json could mitigate the overuse of arrows here? I think if prettier makes it look boilerplate then it's already boilerplate... keeping them in single lines is just hiding the complexity, I guess.

[joshiraez]

And now I realise that this is only half of what #41 should implement. We also need to implement the consumer side of the preferences, since there should be consumers in the Camel plugin that uses these parameters to tune their logics.

So, at least, the camelPreferencesService should provide each method for a corresponding preferences parameter that is supposed to be used from some Camel view elsewhere. In the old Hawtio, these methods were implemented here: https://github.com/hawtio/hawtio-integration/blob/main/plugins/camel/ts/camelHelpers.ts

@phantomjinx @mmelko Are there any places in the Camel views you've implemented where the preferences parameters should be referred to but haven't been yet since the prefs hadn't been implemented yet?

I asked @phantomjinx and he didn't realize if there were any component yet that consumed these settings, but I thought it was cool to ask in the PR as well to see if anyone had any more data. I'll leave the load functions for each preference as before but I'll modify the rest of "loads" to just use an object (I feel like in this case I can do a little more clever coding to ensure that we are always loading a whole object and making sure we don't overwrite any keys, should be simple enough). They should be readily available if something has to consume it :)

[joshiraez]

• Right now values are being saved to local storage and little else. Would like to get your input in if any components already exist to put the wiring on them.

Yes, that's the latter half of the task. But it can be broad and some may be even yet to be implemented, we can work on it in a separate pull req. In this pull req, I think it's ok so long as the camelPreferencesService provides the getter methods for all parameters.

I'll leave all the "load" functions as they are interfaced for now so they are available to be consumed. I'll create a separate issue after this one is merged so we can keep track this is yet to be implemented in the components.

• Reset button. Should I add it? Home preferences didn't have a Reset button before and it has it, same for Jolokla settings. I guess it's a yes but wanted to confirm.

No, you don't need it. As I explained before, we only need two reset buttons in the entire Hawtio console. One is to reset every setting except connections, and the other is to reset connections specifically. That's why I said localStorage.clear() is enough for the former reset button. Probably you're still misunderstanding something.

Home preferences didn't have a Reset button before and it has it, same for Jolokla settings.

I don't see what it means. The old Hawtio has been having the two buttons and nothing changes at this point. One thing to note is that the old General and Reset are refactored into single Home prefs, and Jolokia and Connect are refactored into Connect, because the old prefs were rather cluttered with prefs with sparse options.

oooooooooooooooooooooh, ok. Now I get it.

• Hint icons. I saw we dropped this from other preferences pages. Should I file a separate issue to implement them on all preferences pages?

Good point. Yes, please file another issue and let's tackle it separately. I agree it's good idea to restore the hints.

Done. #195 for traceability

• Overuse of arrow functions on the component: trying to reduce the amount of duplicated code for type checking, I had to use arrow functions because I lose the "this" when passing them as parameters. Is not too much boilerplate... until prettier comes up and puts everything in different lines :(. If I remember that the correct pattern to avoid that was to store this in a const variable in the class, but typescript seemed to not like it.

Possibly my suggestion above to consolidate parameters into a single json could mitigate the overuse of arrows here? I think if prettier makes it look boilerplate then it's already boilerplate... keeping them in single lines is just hiding the complexity, I guess.

It "should" in some cases. I'm going to be a bit more clever and just have a single function for the update of each input parameter instead of having them one by one. Most of the boilerplate comes from it actually, even though is configuration code.

Btw, the issue about using arrow functions on the onDidSomethingChange functions and the issue about using arrow functions on the Checkbox components are different.

The onDidSomethingChange functions will overwrite the this if I pass the function as argument :( (rather, the this is always the one on the environment the function is created. Using an arrow function ensures that the this is the one of the context where the arrow function was created) which is anoying, but is a JS thing.

The checkbox one is about checkbox onChange prop having a different type signature than other inputs.

Just fyi as I didn't put it very clearly when I made that comment

Thank you for your input @tadayosi I'll be refinining this during the following hours

[joshiraez]

Also, something that I forgot to add yesterday pertaining to #173 . This PR doesn't has the bug where the input gets unfocused every time I type. I'll be focusing on finishing this PR by now but I'll keep an eye and try to extract why in this case it works and not in the other component (when they are essentially the same as far as I see)

[joshiraez]

Well I just discovered that keyof exists and can be used as a type in a function signature and that makes the code SOOOO MUCH CLEANER. Finishing it up soon and re-requesting a review

[joshiraez]

@tadayosi Refactored the class to be stored in an object. I optimized to reduce boilerplate and with the keyofoperator I was able to ensure type safety as well so I'm very happy.

There are some places that might find a little unorthodox but it should make the code more legible and avoid the overuse of arrow functions. Tell me your thoughts :)

[joshiraez]

@tadayosi Everything has been addressed. Feel free to close the conversations when you check everything is ok :)