-
Notifications
You must be signed in to change notification settings - Fork 2
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
add keyboard shortcuts readme #207
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Flags with carried forward coverage won't be shown. Click here to find out more. |
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.
It seems like this is mostly AI generated. I tried to read it word by word to make sure it is not missing/hallucinating anything, but it would be nice if you double check it word by word as well as there is too much hallucination and I may have missed something.
Co-authored-by: Sassan Haradji <[email protected]>
Co-authored-by: Sassan Haradji <[email protected]>
Co-authored-by: Sassan Haradji <[email protected]>
Co-authored-by: Sassan Haradji <[email protected]>
Co-authored-by: Sassan Haradji <[email protected]>
@sassanh go it. removed the advanced table. I might need to rebase this before merge but I will wait for the build to finish. It should be okay since there is no code change. |
@sassanh Can you please take a quick look at the readme. I think it should be ready to merge. I applied your comments on it. |
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.
Thanks! Looks very good, I added a few comments regarding consistency and improving readability in the rendered markdown.
|
||
### Screenshots | ||
**Location**: `/opt/ubo/screenshots/` | ||
**Format**: PNG image files |
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.
Unfortunately single line break doesn't show up in the rendered markdown, so these lines are all rendered in a single line, you need to either end the first line with two spaces, put an explicit
at the end of line or put two new lines.
## Special Commands | ||
| Shortcut | Equivalent on Keyboard | Action | | ||
|----------|---------| ---------| | ||
| HOME + L1 | Backspace + 1 | Take screenshot | |
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.
In the keyboard HOME
doesn't become Backspace
when it is being used as a modifier, it becomes Shift
.
Also for BACK
, the modifier key is Ctrl
.
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.
@sassanh so Shift + 1
= HOME + L1
right?
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.
@zytegalaxy Yes, exactly
|----------|-----------------|---------| | ||
| UP | ARROW_UP, or K | Move selection up | | ||
| DOWN | ARROW_DOWN, or J | Move selection down | | ||
| BACK | ESC, H, ← | Go back one level | |
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.
For the sake of consistency I suggest we either change ARROW_UP
and ARROW_DOWN
in previous lines to ↑
and ↓
or change ←
here to ARROW_LEFT
.
Also again for the sake of consistency in the keyboard column, I suggest we either use all uppercase words and change Backspace
to BACKSPACE
or use capital words and use Arrow Up
and Escape
.
We can also consider using unicode characters like ⟲
/↰
and ⌂
for back and home buttons respectively.
Key on Device | Equivalent on Keyboard | Action |
---|---|---|
UP |
↑/K | Go up |
DOWN |
↓ /J | Go down |
BACK ⟲ |
←/H/Esc | Go back one level |
HOME ⌂ |
Backspace | Return to home menu |
L1 |
1 | Select first item |
L2 |
2 | Select second item |
L3 |
3 | Select third item |
And similarly for other tables
**Location**: `/opt/ubo/snapshots/` | ||
**Format**: JSON files | ||
**Naming**: `ubo-screenshot-xxx.json` | ||
|
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.
To avoid confusion for the user we can state that the content of each snapshot file is the dump of the redux store and it is used for debug purposes mostly.
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.
just for my own understanding, can I right now write a test to get store snap shot (A), recordings (B), and a new snapshot (C) to test a journey? Are we doing this right now or we have the building blocks to do it.
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 have all the building blocks and samples for parts of it.
You can do a recording, copy it next to your test file, run it and use the tooling we have for store snapshots to verify the store snapshots.
The first time you run the test, you should run it with --override-store-snapshots
and it will take a snapshot each line you call store_snapshot.take()
and store them in the filesystem. On consecutive runs of the test (without the override flag) it will take the snapshot and instead of writing it to the filesystem, it will check it against the version written in the filesystem.
We don't have a sample for running the recording in the test, but we have sample for using store snapshots:
tests/integration/test_services.py:52
tests/flows/test_wireless.py:154
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.
I added the suggested clarification and pushed the change.
@sassanh do you know why the image creation step has failed? I guess we can merge without any problems right? |
@zytegalaxy It seems to be an issue in packer-builder-arm that rarely shows itself, it's safe to rerun it manually. |
* add keyboard shortcuts readme * Update ubo_app/services/020-keyboard/shortcuts.md Co-authored-by: Sassan Haradji <[email protected]> * Update ubo_app/services/020-keyboard/shortcuts.md Co-authored-by: Sassan Haradji <[email protected]> * Update ubo_app/services/020-keyboard/shortcuts.md Co-authored-by: Sassan Haradji <[email protected]> * Update ubo_app/services/020-keyboard/shortcuts.md Co-authored-by: Sassan Haradji <[email protected]> * Update ubo_app/services/020-keyboard/shortcuts.md Co-authored-by: Sassan Haradji <[email protected]> * updated readme for keyboard/keypad shortcuts * remove avanced label. * clarify what snapshot means * make key names more consistent * more consistency --------- Co-authored-by: Sassan Haradji <[email protected]>
This PR adds readme file to list keypad shortcuts