search

keymanapp / keyboards

Open Source Keyman keyboards
149 stars 267 forks source link

Naijatype experimental: flicks, v17 & documentation updates #2822

Closed rowbory closed 3 months ago

rowbory commented 3 months ago

I've added a lot of Keyman 17 features taking advantage of flicks and tried to update and improve the documentation. At some point I'll be interested in any advice to take this from 'experimental' to the main repository, if that is helpful. Feedback very much useful. One niggle I have is that I can't see how to display different languages not just the same keyboard name in the menu: IMG_0166

keyman-server commented 3 months ago

Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly.

LornaSIL commented 3 months ago

There seems to be a conflict in the description in the .kps that needs resolving:

<<<<<<< naijatype-experimental
    <Description URL="">Package for keyboard. Lexical models need to be packaged separately.</Description>
=======
    <Description>This keyboard attempts to help you to type any letters used in Nigerian 
orthographies.</Description>
>>>>>>> master

I think you can use the "web editor" that is linked to above to resolve that.

image

LornaSIL commented 3 months ago

If there isn't going to be major redesign of the keyboard, renaming the foldername/filenames, etc, and if you have adequate documentation, we are happy for you to move it to release.

mcdurdin commented 3 months ago

One niggle I have is that I can't see how to display different languages not just the same keyboard name in the menu:

Can you open this as a new feature issue at https://github.com/keymanapp/keyman/issues/new/choose?

keyman-server commented 3 months ago

Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly.

rowbory commented 3 months ago

Another small query. When I was testing installing on some other devices I looked on the Download keyboard site and the version number mentioned (56) was from the previous commit, but the date was from this (not yet accepted) PR and the keyboard served is the older one. Is that expected behaviour? I would have expected it to show the old (accepted) file date not a current in process PR date.

image
keyman-server commented 3 months ago

Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly.

LornaSIL commented 3 months ago

Another small query. When I was testing installing on some other devices I looked on the Download keyboard site and the version number mentioned (56) was from the previous commit, but the date was from this (not yet accepted) PR and the keyboard served is the older one. Is that expected behaviour? I would have expected it to show the old (accepted) file date not a current in process PR date.

The whole build system was recently updated (and every keyboard slightly modified to follow Keyman 17 conventions). But that happened last week sometime, so I think the fact it is showing yesterdays date might be a regression in what is displayed. I looked at another keyboard and see the same thing. @mcdurdin was this intentional?

LornaSIL commented 3 months ago

The keyboard won't build. Here are the error messages:

naijatype.kmn - hint KM07005: The touch layout uses a flick or multi-tap gesture on key K_Q, which is only available on version 17.0+ of Keyman
16:52:12   naijatype.kmn - warn KM020A9: Key "K_LOPT" on platform "phone", layer "numeric_old" does not have the key type "Special" or "Special (active)" but has the label "*Menu*". This feature is only supported in Keyman 14 or later
16:52:12   naijatype.kmn - warn KM020A9: Key "K_LOPT" on platform "phone", layer "symbol" does not have the key type "Special" or "Special (active)" but has the label "*Menu*". This feature is only supported in Keyman 14 or later
16:52:12   naijatype.kmn - warn KM020A9: Key "K_LOPT" on platform "phone", layer "capslock" does not have the key type "Special" or "Special (active)" but has the label "*Menu*". This feature is only supported in Keyman 14 or later
16:52:12   naijatype.kmn - info KM05006: experimental/n/naijatype/source/naijatype.kmn built successfully.
16:52:12   naijatype.kps - info KM05002: Building experimental/n/naijatype/source/naijatype.kps
16:52:12   naijatype.kps - error KM04011: Keyboard file C:\BuildAgent\work\aee9ab97cd908689\keyboards\experimental\n\naijatype\build\naijatype.kmx was not found. Has it been compiled?
16:52:12   naijatype.kps - info KM05007: experimental/n/naijatype/source/naijatype.kps failed to build.
16:52:12   naijatype.kpj - info KM0500C: Project experimental/n/naijatype/naijatype.kpj failed to build.
16:52:13   [/c/BuildAgent/work/aee9ab97cd908689/keyboards] ## build failed

I believe you already converted the project to Keyman 17 format. But, I'm not quite sure about the minimum Keyman version needed. Try updating this statement store(&VERSION) '10.0' in the .kmn file to 15.0: store(&version) '15.0'. Hopefully that will fix it.

keyman-server commented 3 months ago

Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly.

rowbory commented 3 months ago

Interesting. I've bumped the minimum Keyman version required (though perhaps that leaves some users behind?) and have done what I can to remove the warnings. I'm hoping the flick gestures will just be ignored on Keyman < 17, and I don't need to make this Keyman 17 only.

The keyboard won't build. Here are the error messages:

naijatype.kmn - hint KM07005: The touch layout uses a flick or multi-tap gesture on key K_Q, which is only available on version 17.0+ of Keyman
16:52:12   naijatype.kmn - warn KM020A9: Key "K_LOPT" on platform "phone", layer "numeric_old" does not have the key type "Special" or "Special (active)" but has the label "*Menu*". This feature is only supported in Keyman 14 or later
16:52:12   naijatype.kmn - warn KM020A9: Key "K_LOPT" on platform "phone", layer "symbol" does not have the key type "Special" or "Special (active)" but has the label "*Menu*". This feature is only supported in Keyman 14 or later
16:52:12   naijatype.kmn - warn KM020A9: Key "K_LOPT" on platform "phone", layer "capslock" does not have the key type "Special" or "Special (active)" but has the label "*Menu*". This feature is only supported in Keyman 14 or later
16:52:12   naijatype.kmn - info KM05006: experimental/n/naijatype/source/naijatype.kmn built successfully.
16:52:12   naijatype.kps - info KM05002: Building experimental/n/naijatype/source/naijatype.kps
16:52:12   naijatype.kps - error KM04011: Keyboard file C:\BuildAgent\work\aee9ab97cd908689\keyboards\experimental\n\naijatype\build\naijatype.kmx was not found. Has it been compiled?
16:52:12   naijatype.kps - info KM05007: experimental/n/naijatype/source/naijatype.kps failed to build.
16:52:12   naijatype.kpj - info KM0500C: Project experimental/n/naijatype/naijatype.kpj failed to build.
16:52:13   [/c/BuildAgent/work/aee9ab97cd908689/keyboards] ## build failed

I believe you already converted the project to Keyman 17 format. But, I'm not quite sure about the minimum Keyman version needed. Try updating this statement store(&VERSION) '10.0' in the .kmn file to 15.0: store(&version) '15.0'. Hopefully that will fix it.

mcdurdin commented 3 months ago

I've bumped the minimum Keyman version required (though perhaps that leaves some users behind?) and have done what I can to remove the warnings. I'm hoping the flick gestures will just be ignored on Keyman < 17, and I don't need to make this Keyman 17 only.

You shouldn't need to specify the version number; if you leave the store(&version) line out, the compiler can infer the minimum version required (there is one exception, but I don't think it applies to this keyboard). Flicks and other gestures do not impact the minimum version -- they will just not be accessible on earlier versions of Keyman.

mcdurdin commented 3 months ago

But that happened last week sometime, so I think the fact it is showing yesterdays date might be a regression in what is displayed. I looked at another keyboard and see the same thing. @mcdurdin was this intentional?

This is a side-effect of every release and experimental keyboard in the repository being touched in order to move the .keyboard_info metadata into the .kps file. Avoiding reporting of this change would require a non-trivial change to the build process, as currently the build uses the data in the git repository to determine the last date of modification (code). I opted not to try because I didn't think it was important enough, but we could give it a go if other people think it is important.

keyman-server commented 3 months ago

Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly.

keyman-server commented 3 months ago

Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly.

rowbory commented 3 months ago

This is a side-effect of every release and experimental keyboard in the repository being touched in order to move the .keyboard_info metadata into the .kps file.

What struck me as odd though was that the version number reported (56) was itself last modified some weeks ago, so the 'last modified' and 'latest keyboard version number' were not connected in any way. But I accept this is only likely to show up in the rare circumstance that an update has been pushed and a PR is active but not yet accepted. Would it be possible to get the last-modified from the same revision that the 'Latest Version' comes from? Not worth it if there's lots of work to do that.

mcdurdin commented 3 months ago

so the 'last modified' and 'latest keyboard version number' were not connected in any way

Yes, because we didn't update version numbers, so this only impacts the metadata shown on the website.

However, this discussion prompted keymanapp/keyman#11793, which should be straightforward to implement, and would mitigate the problem, so thank you!

keyman-server commented 3 months ago

Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly.

keyman-server commented 3 months ago

Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly.

keyman-server commented 3 months ago

Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly.

keyman-server commented 3 months ago

Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly.

keyman-server commented 3 months ago

Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly.

keyman-server commented 3 months ago

Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly.

keyman-server commented 3 months ago

Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly.

LornaSIL commented 3 months ago

I can't find the review I did yesterday. I must have closed it without saving. Please open your kpj project in developer and compile your .kmn file. If you can address some of those warning it might be useful.

Then, go to the .kps and try to compile. You will see these messages:

naijatype.kps - info KM05002: Building naijatype.kps
naijatype.kps - error KM04003: File C:\SRC\GitHub\keyboards\experimental\n\naijatype\source\Andika-Regular.ttf does not exist.
naijatype.kps - error KM04003: File C:\SRC\GitHub\keyboards\experimental\n\naijatype\source\icons_pictures\naijatype24_140x250.jpg does not exist.
naijatype.kps - error KM04003: File C:\SRC\GitHub\keyboards\experimental\n\naijatype\source\welcome\default.jpg does not exist.
naijatype.kps - error KM04003: File C:\SRC\GitHub\keyboards\experimental\n\naijatype\source\welcome\touch_ios_install.jpg does not exist.
naijatype.kps - error KM04003: File C:\SRC\GitHub\keyboards\experimental\n\naijatype\source\welcome\shift_layer.png does not exist.
naijatype.kps - error KM04003: File C:\SRC\GitHub\keyboards\experimental\n\naijatype\source\welcome\default.png does not exist.
naijatype.kps - error KM04003: File C:\SRC\GitHub\keyboards\experimental\n\naijatype\source\welcome\numbers_key.png does not exist.
naijatype.kps - error KM04003: File C:\SRC\GitHub\keyboards\experimental\n\naijatype\source\welcome\shift_key.png does not exist.
naijatype.kps - info KM05007: naijatype.kps failed to build.

You've either renamed some of your files or deleted them. Also, the font name is not correct. Either put Andika-Regular.ttf in your commit or change the .kps to reference AndikaAfr-R.ttf.

The errors must be resolved. The warning are related to not having a minimum Keyman version in your header in the .kmn so I guess that's okay.

keyman-server commented 3 months ago

Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly.

keyman-server commented 3 months ago

Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly.

rowbory commented 3 months ago

Thanks. I switched to Andika (full) and seem to have left behind the non-text updates when I staged from my development folder to the local repo. Thanks for noticing that.

keyman-server commented 3 months ago

Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly.

keyman-server commented 3 months ago

Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly.

rowbory commented 3 months ago

OK, I now have a new regime of building in my local repo before I push. Should I be worried if I push and then see 'TeamCity build failed'? Sorry for taking up so much of your time @LornaSIL

LornaSIL commented 3 months ago

I know it's confusing. No, you don't need to be concerned that it says "Some checks were not successful". We don't automatically kick off a build until we've done preliminary checking so that our build agent doesn't have to work too hard. When there are a lot of changes to a keyboard I'll generally do a build on my system first to make sure it builds for me before I ask our agent to do a build. In this case, it builds for me, so I'll go ahead and see if our system is also happy with it :)