added cameron/frontend_v4#33
added cameron/frontend_v4#33camerontarget14 wants to merge 3 commits intoAcademySoftwareFoundation:mainfrom
Conversation
Signed-off-by: camerontarget14 <127965975+camerontarget14@users.noreply.github.com>
|
We can go the way of doing it manually but I will say that I am a bit concerned about not using a existing package and global manager. We are leaving some functionality on the table that would be easier to maintain and implement. For example, there is a lot of duplicate logic. Take a look at the long render statments in settings and the check for hotkeys effect. There is a lot of logic that needs to be implemented every time a hotkey is added. For things that are discoverable and configurable, a context provider works best. The way I see this MR, it should implement A) The callbacks for the hotkeys, B) add hotkeysConfig.ts file that has the hostkeys and mapped methods and C) the package implemented. There are some other things the packages offer as well such as displaying the hotkey on tools tips on thier corresponding buttons that we would get for free. There are also some lower level guards agienst conflicting events and focus traps. All that being said, not going to die on hotkey hill XD If there is somthing we are not getting from using an off the shelf package, then we can reimplement. |
|
@jspada200 at a high level, that all makes a lot of sense to me! I'd be happy to take another pass so it's off your plate, but won't be till the weekend.. |
Includes: