-
Notifications
You must be signed in to change notification settings - Fork 94
feat: show macro key assignments on the macro page #3002
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
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
91de325
Show macro key assignments on the macro page.
mondalaci 2f191a5
Address review: keep UI helpers in uhk-web and slim the macro header.
mondalaci 6deda76
Tweak Used on spacing and keep macro popover controls on one line.
mondalaci 8ce6cb7
fix: fine tuning
ert78gb 4932d07
Load default keymap on app start for macro key labels.
mondalaci File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| --- | ||
| description: Prefer options objects when a function needs more than three parameters | ||
| alwaysApply: true | ||
| --- | ||
|
|
||
| # Function parameters | ||
|
|
||
| Functions and methods with more than three parameters are hard to maintain. Prefer an options/payload object once the call needs four or more values. | ||
|
|
||
| ```typescript | ||
| // ❌ BAD | ||
| createKeymapWithMacroAssignment(abbr, name, layerId, moduleId, keyId, macroId) | ||
|
|
||
| // ✅ GOOD | ||
| createKeymapWithMacroAssignment({ abbreviation, name, layerId, moduleId, keyId, macroId }) | ||
| ``` | ||
|
|
||
| Three-or-fewer scalar params are fine when the meaning stays obvious at the call site. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| --- | ||
| description: Prefer Bootstrap and existing styles over new custom CSS | ||
| alwaysApply: true | ||
| --- | ||
|
|
||
| # Prefer existing styles | ||
|
|
||
| Do not add custom CSS when Bootstrap utilities or existing shared styles already cover spacing, typography, and layout. | ||
|
|
||
| ## Prefer | ||
|
|
||
| - Bootstrap utilities: `my-2`, `mb-3`, `me-1`, `small`, `text-nowrap`, `float-end`, … | ||
| - Patterns already used nearby (e.g. `back-to` uses `class="my-2"`) | ||
|
|
||
| ## Avoid | ||
|
|
||
| - New BEM blocks for one-off spacing/font-size/weight that utilities already express | ||
| - Growing component SCSS with unexplained overrides that later confuse deletions | ||
|
|
||
| ```html | ||
| <!-- ❌ BAD --> | ||
| <div class="macro__assignments">…</div> | ||
| <!-- plus .macro__assignments { font-size: 0.875rem; padding-top: 1.25em; } --> | ||
|
|
||
| <!-- ✅ GOOD --> | ||
| <div class="my-2 small">…</div> | ||
| ``` | ||
|
|
||
| Only add custom CSS when no existing utility/shared rule fits, and keep it minimal. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| --- | ||
| description: Prefer smart containers / dumb leaf components with store-side calculations | ||
| alwaysApply: true | ||
| --- | ||
|
|
||
| # Smart / dumb components | ||
|
|
||
| Follow the smart/dumb (container/presentational) pattern already used in this codebase. | ||
|
|
||
| ## Responsibilities | ||
|
|
||
| - **Store / selectors / smart parents**: derive view models, sort/filter/map domain data, combine streams. | ||
| - **Leaf components**: receive `@Input()`s, render them, emit `@Output()`s / dispatch obvious user intents. Avoid reading the store just to compute display data. | ||
|
|
||
| ```typescript | ||
| // ❌ BAD — deep leaf component selects and calculates | ||
| class MacroHeaderComponent { | ||
| constructor(store: Store) { | ||
| store.select(getKeymaps).subscribe(keymaps => this.assignments = map(keymaps)); | ||
| } | ||
| } | ||
|
|
||
| // ✅ GOOD — smart parent selects; leaf renders | ||
| // macro-edit (smart) | ||
| assignments$ = this.store.select(getSelectedMacroKeyAssignments); | ||
|
|
||
| // template | ||
| <macro-header [macro]="macro" [assignments]="assignments$ | async"></macro-header> | ||
|
|
||
| // macro-header (dumb regarding this data) | ||
| @Input() assignments: MacroKeyAssignmentViewModel[] = []; | ||
| ``` | ||
|
|
||
| Existing leaf components that already inject `Store` only to `dispatch` actions are acceptable when inherited; do not expand them with new selectors or calculation logic. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| --- | ||
| description: Keep UI and presentation code out of uhk-common | ||
| alwaysApply: true | ||
| --- | ||
|
|
||
| # Package boundaries: uhk-common vs uhk-web | ||
|
|
||
| `uhk-common` is for shared, UI-agnostic domain logic (config serializers, USB helpers, pure data transforms). Do **not** put presentation-oriented helpers there. | ||
|
|
||
| ## Belong in `uhk-web` (or uhk-agent), not `uhk-common` | ||
|
|
||
| - Display labels, breadcrumbs, menu grouping, view-model mappers | ||
| - Anything that formats data for Angular templates | ||
| - Code that only Agent UI consumers need, even if it looks "pure" | ||
|
|
||
| ## Belong in `uhk-common` | ||
|
|
||
| - Config/device models and serialization | ||
| - Algorithms that both Electron main and renderer need without UI assumptions | ||
|
|
||
| ```typescript | ||
| // ❌ BAD — lives in uhk-common only because tests were convenient there | ||
| export function findMacroKeyAssignments(...): MacroKeyAssignment[] { /* for macro page UI */ } | ||
|
|
||
| // ✅ GOOD — presentation helper next to the UI that uses it | ||
| // packages/uhk-web/src/app/util/find-macro-key-assignments.ts | ||
| ``` | ||
|
|
||
| If you want unit tests for UI helpers and `uhk-web` has no runner yet, still put the code in `uhk-web` and add tests there; do not grow UI surface area in `uhk-common` for convenience. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1 change: 0 additions & 1 deletion
1
packages/uhk-web/src/app/components/macro/header/macro-header.component.scss
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,3 @@ | ||
|
|
||
| .macro { | ||
| &__remove { | ||
| font-size: 0.75em; | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
9 changes: 9 additions & 0 deletions
9
packages/uhk-web/src/app/models/macro-key-assignment-view-model.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| import { LayerName } from 'uhk-common'; | ||
|
|
||
| export interface MacroKeyAssignmentViewModel { | ||
| keymapAbbreviation: string; | ||
| layerId: LayerName; | ||
| moduleId: number; | ||
| keyId: number; | ||
| label: string; | ||
| } |
1 change: 1 addition & 0 deletions
1
packages/uhk-web/src/app/models/navigate-to-module-settings-payload.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We load the default configuration into the store when opening the
Add Keymapmenu.I do it to save some memory because it is not frequently used feature. We could load it and app start and refresh when new device is connected, but I haven't did it, because little bit confusing me to see the macro assigned on a non qwerty keymap and we so the qwerty key name.
For example I assign the macro to the Dvorak O that is Qwerty S and I see
Dvorak for PC -> Base -> Sinstead ofDvorak for PC -> Base -> O