Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding shortcut keys to perform certain actions like Build , Pin/Unpin editor, Close/Open editor, Run and Reset #187

Open
surajkumar-sk opened this issue Apr 8, 2022 Discussed in #185 · 7 comments
Labels
on-hold Issues on hold

Comments

@surajkumar-sk
Copy link

surajkumar-sk commented Apr 8, 2022

Discussed in #185

Originally posted by surajkumar-sk March 16, 2022
While learning about what different syntax does in musicblock v4 I had to edit and run the code a lot of times, I kept resetting the block and building the code for almost every 5 sec. It was fine moving my mouse to click the buttons in the beginning but after few attempts, It started getting frustrating to hover and click on build, run and reset. I as a Developer will probably not build, run and reset many times. Buts kids and teachers using this website might have to click on buttons more often. Adding shortcuts to buttons would make it easier to perform actions. we could add a hover text on buttons showing the shortcut keys. Changes in code wouldn't be much but adding this can make accessibility of buttons a lot easier for frequent learners or teachers.

shortcut for run - alt + r
shortcut for stop - alt + s
shortcut for reset - ctrl + alt + r
shortcut for opening or closing editor - ctrl + alt + e
shortcut for pin or unpin - alt + p
shortcut for build - alt + b
shortcut for help - alt + h

I would like to implement this changes as my good first issue and begin contributing to the project.

@surajkumar-sk
Copy link
Author

surajkumar-sk commented Apr 10, 2022

@meganindya In the last conversation we had you specified your concern about code editor being updated which might make any code I implement today irrelavent.
But there are a lot of buttons outside code editor for which shortcut keys can be added which most probably won't be affected by code editor. After puting a bit of thought into this I realized the buttons are spread across a lot of files some are created at the top hierarcy files and some are in low hierarcy files and there are a lot of buttons which will get added in the futures at different levels. If the shortcut eventListeners are adding during the setup of these buttons, it would be very hard to keep track of buttons and their shortcut keys which might lead to overlaping shorcutKeys and make adding shortcut keys a bit harder later on.

At core to solve these problems we need to have all the shortcut keys and buttons linked to them in a single file. There might be a lot of eventlistener function running for a single click so rather that combining all those functions and runnig them when the shortcut key is pressed we could just emit a click event for the button whose shortcut key is pressed. By doing this we make a global EventListener independent of type of button. The only information this EventListener function would need is id name of button and shortcut key.

code flow -

  1. we need to add id's to buttons for which we need to add shortcut keys.
  2. we create a JSON file shortcuts.json containing array of buttons along with the shortcut key for that button
    ex -
    [
     {
       id:'toolbar-items-run-button',
       shotcutkey: 'alt + r'
     },
     {
       id:'toolbar-items-stop-button',
       shotcutkey: 'alt + s'
     }
    ]
  1. add an eventListener on windows for keyDown and keyUp and then loop through the above JSON data to determine which button's click should be emmited for a specific shortcut key. This eventListener will be added after the mount and setup process.
    code -

    let keys = [];
    
    fetch("./shortcuts.json")
    .then(response => {
        let shortcuts = response.json();
        function emitEventForShortcut(){
            shortcuts.map((shotcut)=>{
               let shortcutArray = shortcut.shortcutkey.split('+'); 
               if(shortcutArray.length == keys.length){
                    let keysCount = shortcutArray.filter((sk) =>{
                        let exists = keys.filter((key)=>{
                            if(key == sk){
                                return true;
                            }
                            return false;
                        })
                        if(exists.length){
                            return true;
                        }
                    });
                    if(keysCount.length == keys.length){
                        let btn = document.getElementById(shortcut.id);
                        btn.dispatchEvent(new Event('click'));
                    }
               }
            })
        }
        window.addEventListener('keydown',(e)=>{
            keys.push((e.key).toLowerCase());
            emitEventForShortcut();
        });
        window.addEventListener('keyup',(e)=>{
            let eventKey = (e.key).toLowerCase();
            keys= keys.filter((key) =>{
                if(key == eventKey){
                    return false;
                }
                return true;
            });
            keys.push((e.key).toLowerCase());
            emitEventForShortcut();
        })
    })

above code follows js rules, while writing the actual code I'll make sure to write as per ts.

having all shorcut keys in a single file and implementing emitevent as mentioned above, makes it easy to maintain and expand.

@meganindya if everything is ok, I would like to start working on a PR for this

@surajkumar-sk
Copy link
Author

surajkumar-sk commented Apr 11, 2022

@meganindya , After your previous Msg, I realized I need to make these shortcuts without id's of div rather use the reference of the button that we create when that button is initialized this will convert this into top down approach. and for achieving modularity we create a blank array above the emitEventForShortcut function and two functions for adding btnRef and shortcut keys and removing them from array rather than a json file. I would also suggest to add a text file to store all the shortcuts and button they are linked to, just to keep a track of shortcut buttons.

code flow -

  1. making a blank shortcutsKeys array and making functions to add and remove data from the array. this array should be placed on the top level of hierarchy tree.
let shortcutKeys = [];
//keydata:{btn:btnRef,shortcut:"alt+x"}
function addShortcutKeys(keyData){
  keyData.shortcut = keyData.shortcut.replaceAll(' ',"");
  // to prevent any duplicate values
  let testshortcutKeys = shortcutKeys.filter((shortcutKey)=>{
    if(shortcutKey.btn == keyData.btn) return false;
    else if(shortcutKey.shortcut == keyData.shortcut) return false;
    return true;
  });
  if(testshortcutKeys.length != shortcutKeys){
    throw "Duplicate shortcuts being assigned please recheck the values"
  } else{
    shortcutKeys.push(keyData);
  }
}
function removeShotcutKeys(keyData){
  keyData.shortcut = keyData.shortcut.replaceAll(' ',"");
  shortcutKeys = shortcutKeys.filter((shortcutKey) =>{
    if(shortcutKey.btn == keyData.btn && shortcutKey.shortcut == keyData.shortcut){
      return false;
    } 
    return true;
  });
  if(testshortcutKeys.length == shortcutKeys){
    throw "No shortcut key for that button found";
  }
}
  1. calling these function when the buttons get mounted,
    ex - run ,stop and reset button -
export function setup(): Promise<void> {
    return new Promise((resolve) => {
        requestAnimationFrame(() => {
            const buttons = getButtons();
            buttons.run.addEventListener('click', () => {
                const crumbs = getCrumbs();
                if (crumbs.length !== 0) run(getCrumbs()[0].nodeID);
                updateState('running', true);
                setTimeout(() => updateState('running', false));
            });
            buttons.stop.addEventListener('click', () => {
                updateState('running', false);
            });
            buttons.reset.addEventListener('click', () => {
                updateState('running', false);
            });
            addShortcutKeys({btn:buttons.run,shortcut:"alt+r"});
            addShortcutKeys({btn:buttons.stop,shortcut:"alt+s"});
            addShortcutKeys({btn:buttons.reset,shortcut:"alt+r"});

            resolve();
        });
    });
}
  1. now after running all the setups we can add eventlistener for keyup and keydown and run emitEventForShortcut function on shortcutKeys arrays. Because the array will be dynamic modularity , couple-decouple can be achieved.
    Same code as previous comment code.
let shortcuts = shortcutKeys;
let keys = [];
function emitEventForShortcut(){
            shortcuts.map((shotcut)=>{
               let shortcutArray = shortcut.shortcut.split('+'); 
               if(shortcutArray.length == keys.length){
                    let keysCount = shortcutArray.filter((sk) =>{
                        let exists = keys.filter((key)=>{
                            if(key == sk){
                                return true;
                            }
                            return false;
                        })
                        if(exists.length){
                            return true;
                        }
                    });
                    if(keysCount.length == keys.length){
                        let btn = shortcut.btn.current;
                        btn.dispatchEvent(new Event('click'));
                    }
               }
            })
        }
        window.addEventListener('keydown',(e)=>{
            keys.push((e.key).toLowerCase());
            emitEventForShortcut();
        });
        window.addEventListener('keyup',(e)=>{
            let eventKey = (e.key).toLowerCase();
            keys= keys.filter((key) =>{
                if(key == eventKey){
                    return false;
                }
                return true;
            });
            keys.push((e.key).toLowerCase());
            emitEventForShortcut();
        })
    })

@surajkumar-sk
Copy link
Author

surajkumar-sk commented Apr 12, 2022

@meganindya After your Previous Message I think I understand how you want this project to be structured.

You want to achieve complete modularity which means there shouldn't be any piece of code outside the module that manipulates elements of the current module without an interface or instructions present in the current module. You gave the example of mountHook(that stuff was great). I just need to create a function for running eventListener on windows to trigger shortcuts but lets other modules call this function and let them decide what shortcut to pass and what to do when that shortcut is run. Something similar to what you did with run and reset button with mountHook.

code flow -

  1. creating a global function to accept a shortcut key and a callback function and add them to the list of shortcut and callback functions amd then run a loop on the list inside emitEventForShortcut() and execute all the callback functions when a shortcut is triggered.

var list = [];

function runShortcut(shortcut,callbackFun){
  list .push({shortcut,callbackFun});
}
 let keys = [];

function emitEventForShortcut(){
     list.map((S_C) => {
        let shortcut = S_C.shortcut
        let shortcutArray = shortcut.split('+'); 
        if(shortcutArray.length == keys.length){
            let keysCount = shortcutArray.filter((sk) =>{
                let exists = keys.filter((key)=>{
                    if(key == sk){
                        return true;
                    }
                    return false;
                })
                if(exists.length){
                    return true;
                }
                return false;
            });
            if(keysCount.length == keys.length){
                S_C.callbackFun();
            }
        }});
    }

window.addEventListener('keydown',(e)=>{
        keys.push((e.key).toLowerCase());
        emitEventForShortcut();
    });

window.addEventListener('keyup',(e)=>{
        let eventKey = (e.key).toLowerCase();
        keys= keys.filter((key) =>{
            if(key == eventKey){
                return false;
            }
            return true;
        });
        keys.push((e.key).toLowerCase());
        emitEventForShortcut();
    })
}

  1. Now the above function provides an interface for running a function when a specific shortcut key is pressed.
    this function can be called for any component and the component decides what shortcut key to run and what to do when that key is executed.
export function setup(): Promise<void> {
    return new Promise((resolve) => {
        requestAnimationFrame(() => {
            const buttons = getButtons();
            buttons.run.addEventListener('click', () => {
                const crumbs = getCrumbs();
                if (crumbs.length !== 0) run(getCrumbs()[0].nodeID);
                updateState('running', true);
                setTimeout(() => updateState('running', false));
            });
            buttons.stop.addEventListener('click', () => {
                updateState('running', false);
            });
            buttons.reset.addEventListener('click', () => {
                updateState('running', false);
            });

--------------------start here -------

            runShortcut('alt+r',()=>{
              // add any condition to prevent dispatch incase it's not in focus.
              buttons.run.dispatchEvent(new Event('click'));
            }

            runShortcut('alt+s',()=>{
              // add any condition to prevent dispatch incase it's not in focus.
              buttons.stop.dispatchEvent(new Event('click'));
            }

------------------------end------------------       
            resolve();
        });
    });
}

I think this should do the trick.

@surajkumar-sk
Copy link
Author

surajkumar-sk commented Apr 17, 2022

@meganindya Last time we had a chat, you approved the above method but your problem was you didn't want to run a loop for executing the shortcuts because browsers might not handle loops very well. So we can create a hash map with key as Shortcut key and value as the list of call backs that'll run when the shortcut key is pressed .
A PR for this issue will create an interface for other components to add shortcut key event and run a function containing actions to be taken when that shortcut key is triggered.
Code flow :

let shortcutMap = {};
// shortcutsMap:{
    // "alt+w":[callback1,callback2],
    // "alt+v":[callback1]
// }
function runShortcut(shortcut,callbackFun){
    shortcut = shortcut.toLowerCase().replaceAll(" ","")
    shortcutsMap[shortcut].push(callbackFun);
}

function addEvenlisternerOnWindow(){
  let keys = [];

  function emitEventForShortcut(){
    //we make permutations of all possible strings with the keys inside keys[]
    // and pass them to shortcutsMap as key to find if that shortcut exists. this
    // way we will avoid looping through shortcutsMap but also get the required functions linked
    // with this shortcut key.
    // keys pressed at once will be very less so this loop will be very fast.
    let combinationsKeys=[];
    keys.map((key1)=>{
        keys.map((key2)=>{
            combinationsKeys.push(`${key1}+${key2}`);
        });
    })

    combinationsKeys.map((key)=>{
        let functionsArr = shortcutMap[key];
        if(functionsArr){
            functionsArr.map((fun)=>{
                fun();
            });
            break;
        }
    });
   }

  window.addEventListener('keydown',(e)=>{
       keys.push((e.key).toLowerCase());
       emitEventForShortcut();
   });

  window.addEventListener('keyup',(e)=>{
       let eventKey = (e.key).toLowerCase();
       keys= keys.filter((key) =>{
           if(key == eventKey){
               return false;
           }
           return true;
       });
       keys.push((e.key).toLowerCase());
       emitEventForShortcut();
   })
 }
}
  1. Now the above function provides an interface for running a function when a specific shortcut key is pressed.
    this function can be called for any component and the component decides what shortcut key to run and what to do when that key is executed.
runShortcut('alt+r',()=>{
   // add any condition to prevent dispatch incase it's not in focus.
   buttons.run.dispatchEvent(new Event('click'));
}

runShortcut('alt+s',()=>{
   // add any condition to prevent dispatch incase it's not in focus.
   buttons.stop.dispatchEvent(new Event('click'));
}

I think you'll approve of this.

@meganindya
Copy link
Member

I understand this but do discuss how you want to distribute/orchestrate the behaviour across components in general and code modules in specific

@surajkumar-sk
Copy link
Author

surajkumar-sk commented Apr 29, 2022

When I proposed this idea, the main idea behind this was that the user doesn't have to use his mouse every time user run his code, using mouse becomes frustrating after few attempts. Unless we introduce drag and drop blocks, user will be able to perform every action with keyboard. Usually every browser has a lot of shortcuts predefined which we need to make sure not to use. "alt+any alphabet" seems to have no effect on browsers so I propose we use the combination of alt with alphabets, if the same alphabet needs to repeated we use shift+alt. @meganindya when we had a talk last time you specified having just r key to run the code but when typing coding user might press 'r' for inputting something and to make 'r' shortcut work we need to write something saying run shortcuts only when code editor is not in focus, but this way the user still has to use mouse to move the focus away from code editor. So alt+alphabet/shift+alt+alphabet is better.

The general idea behind making shortcut is, we have a hashmap with key values as shortcut key as "alt+a" and value will be array of functions that needs to run when that shortcut is triggered. we will have an interface function that will add shortcuts and functions into hashmap, this interface function can be used by components to add shortcuts and instructions on what to do. This way individual component has complete control on which shortcuts to add and what to do when shortcuts are run. after all the shortcuts and functions get added to hashmap we add an eventlistener on window and run all the functions of the array when a shortcut is triggered.

Implementation -

  1. Creating a blank hashmap for adding shortcuts and functions array in ./view/index.ts and export it.
  2. Creating an interface function inside ./view/index.ts which will add items to hashmap. we have an interface for creating items in this file , so it seems appropriate to add this interface function in this file. Code for this function runShortcut is written in above comment '.
  3. we create function specifying what to do on shortcut trigger inside ./src/component/componentName/index.js .
  4. In setup function of all the components we call the interface function and pass shortcutkey and function as arguments. the interface function will add the shortcut and function into the hashmap.
  5. after setup of all components gets finished and all the shortcuts get added into hashmap. we add an eventlistener on windows inside setupComponent() after all the iterations gets over. the function adding eventlistener addEvenlisternerOnWindow is in above comment.

I understand that there are gonna be lot of changes when code editor gets updated so for this pr i would like to create an interface function, eventlistener adding function and shortcuts just for code editor pin and unpin button as a POC

I hope this comment makes everything clear. I'll think of better names for functions later(suggestions are welcome).

@meganindya
Copy link
Member

Please make a POC of the framework. For now, just implement run and stop as alt/opt + r and alt/opt + x. Encapsultate your work in src/view/interaction.ts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
on-hold Issues on hold
Projects
Status: 💡 Ideas
Development

No branches or pull requests

2 participants