luc-github / ESP3D-WEBUI

A Web UI for ESP8266 or ESP32 based boards connected to 3D printers / CNC
GNU General Public License v3.0
756 stars 305 forks source link

3.0 keyboard remaping, log for Dev and User self-help. Remove unrequired NOP check. #286

Closed aaronse closed 1 year ago

aaronse commented 1 year ago

@luc-github, thanks again for implementing the UI for configuring key mappings. Looks good to me!

How does User unassign key shortcut for a command from within the settings UI. e.g. Disable a command from having key shortcut? Not a big deal, I think it's ok to tell people to edit the preferences.js file directly for that unlikely situation.

Collapsible key mapping section might help Users realize there's other settings in the jog panel.

luc-github commented 1 year ago

I have added the missing clear short key feature There is no collapse list feature currently, all is in scrolling list, but it looks I messed up the preferences.json(s) files modifications after modifications and so order was indeed incorrect, I have fixed it for 3DPrinter

I do no put any console output for user - that is why I removed it , because it increase final package size for ususally very limited benefit, the current console output are for current dev to be sure panels are not over refreshed and they will be removed in beta, I only use console output for errors because errors may need more informations

I am not sure to understand the benefit of using an object (getKeyVal ) instead variable (keyval ) if not re-used, can you explain ?

Let me know what you think

aaronse commented 1 year ago

I have added the missing clear short key feature

Nice. Will pull and take a look.

I do no put any console output for user - that is why I removed it , because it increase final package size for ususally very limited benefit, the current console output are for current dev to be sure panels are not over refreshed and they will be removed in beta, I only use console output for errors because errors may need more informations

I see. Log seems useful to help User when troublshooting. I will remove for now.

I am not sure to understand the benefit of using an object (getKeyVal ) instead variable (keyval ) if not re-used, can you explain ?

getKeyVal fn was also being used in the console.log call. I simplified the conditional logic when refactoring.