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

added 810e #2215

Merged
merged 3 commits into from
Mar 31, 2024
Merged

added 810e #2215

merged 3 commits into from
Mar 31, 2024

Conversation

yulei
Copy link
Contributor

@yulei yulei commented Mar 30, 2024

via support for 810e keyboard

Description

add 810e on via

QMK Pull Request

qmk/qmk_firmware#22883

Checklist

  • The VIA support for this keyboard is MERGED in QMK master already (MANDATORY)
  • The VIA definition follows the guide here: https://caniusevia.com/docs/layouts
  • I have a V3 JSON version for this keyboard definition.(MANDATORY)
  • I have tested this keyboard definition using VIA's "Design" tab.
  • I have tested this keyboard definition with firmware on a device.
  • I have assigned alpha keys and modifier keys with the correct colors.
  • The Vendor ID is not 0xFEED

@Cipulot
Copy link
Collaborator

Cipulot commented Mar 30, 2024

@yulei some considerations with your JSON:

  • the key color convention described in the docs was not followed
  • added stepped caps keys as a layout option, which is not allowed as per doc requirements (also a simple test would have shown that stepped caps aren't rendered as such). Furthermore, it shares the same row/col, so no need to make it an option
  • there is no need to replicate the spacebar key in the alternative layout, the key is the same and in the same position

image

I've moved to uncheck the items in the PR checklist that evidently were not followed.
Apply JSON file formatting too once the above is addressed.

@Cipulot Cipulot added the needs work The PR needs fixes/ things need to be addressed label Mar 30, 2024
@yulei
Copy link
Contributor Author

yulei commented Mar 31, 2024

@yulei some considerations with your JSON:

  • the key color convention described in the docs was not followed
  • added stepped caps keys as a layout option, which is not allowed as per doc requirements (also a simple test would have shown that stepped caps aren't rendered as such). Furthermore, it shares the same row/col, so no need to make it an option
  • there is no need to replicate the spacebar key in the alternative layout, the key is the same and in the same position

image

I've moved to uncheck the items in the PR checklist that evidently were not followed. Apply JSON file formatting too once the above is addressed.

the JSON file was updated according to those issues, thanks

v3/neson_design/810e.json Outdated Show resolved Hide resolved
@Cipulot Cipulot merged commit c585bd6 into the-via:master Mar 31, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs work The PR needs fixes/ things need to be addressed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants