Skip to content

Commit

Permalink
feat(web): address code review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
ermshiperete committed Sep 6, 2024
1 parent 59019cc commit bc46458
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 11 deletions.
7 changes: 2 additions & 5 deletions core/include/keyman/keyman_core_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -1179,13 +1179,10 @@ km_core_status km_core_keyboard_load_from_blob(void* blob, size_t blob_size,
: In the event an internal memory allocation fails.
`KM_CORE_STATUS_IO_ERROR`
: In the event the keyboard file is unparseable for any reason
: In the event the keyboard blob is unparseable for any reason
`KM_CORE_STATUS_INVALID_ARGUMENT`
: In the event `keyboard` is null.
`KM_CORE_STATUS_OS_ERROR`
: Bit 31 (high bit) set, bits 0-30 are an OS-specific error code.
: In the event one of the required parameters is null.
-------------------------------------------------------------------------------
Expand Down
12 changes: 6 additions & 6 deletions core/src/layout.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,9 @@ enum keyboard_layout_key_type {
*/
struct keyboard_layout_key {
/** key id */
std::u16string id; // ??? perhaps necessary for special keys, Enter, etc? or can we get that from virtualKey?
std::u16string id; // TODO-WEB-CORE: perhaps necessary for special keys, Enter, etc? or can we get that from virtualKey?
/** the virtual key code */
int virtualKey; // ??? do we need this? both id and virtualKey? Or just one of them?
int virtualKey; // TODO-WEB-CORE: do we need this? both id and virtualKey? Or just one of them?
/** text to display on key cap */
std::u16string display;
/** hint e.g. for longpress */
Expand Down Expand Up @@ -106,7 +106,7 @@ struct keyboard_layout_key {
*/
struct keyboard_layout_row {
/** row id */
int id; // ??? do we need this? Web has it (`TouchLayoutRow`)
int id; // TODO-WEB-CORE: do we need this? Web has it (`TouchLayoutRow`)
/** keys in this row */
std::vector<keyboard_layout_key> keys;
};
Expand All @@ -118,7 +118,7 @@ struct keyboard_layout_layer {
/** layer id */
std::u16string id;
/** layer modifiers */
// ??? we added this during our discussion, but Web doesn't have it.
// TODO-WEB-CORE: we added this during our discussion, but Web doesn't have it.
// Should be an enum if it's needed.
int modifiers; //? 0 = default, n = shift, etc. -1 = unspecified?
/** rows in this layer */
Expand All @@ -136,7 +136,7 @@ struct keyboard_layout_platform {
/** layers for this platform */
std::vector<keyboard_layout_layer> layers;

// ??? Do we need these:
// TODO-WEB-CORE: Do we need these:
// Web additionally has:
// - font (should be in CSS; we have it in `keyboard_layout`)
// - fontsize (should be in CSS; we have it in `keyboard_layout`)
Expand All @@ -155,7 +155,7 @@ struct keyboard_layout {
/** font face name to use for key caps*/
std::string fontFacename;
/** font size to use for key caps */
int fontSizeEm; // ??? em? px? something else?
int fontSizeEm; // TODO-WEB-CORE: em? px? something else?
};


Expand Down

0 comments on commit bc46458

Please sign in to comment.