divvun / giellakbd-ios

Open source reimplementation of iOS keyboard with localisation support
Apache License 2.0
29 stars 5 forks source link

12" iPad keyboard layout issues #222

Closed snomos closed 11 months ago

snomos commented 12 months ago

SME 12" keyboard:

Skjermbilde 2023-09-18 kl  11 21 54

Apple nynorsk 12" keyboard:

Skjermbilde 2023-09-18 kl  11 16 52

dylanhand commented 11 months ago

Build 0.2.6 (127) of Divvun Dev Keyboards has a preliminary fix for the height issue. @snomos can you test?

I think I discovered a new bug as well, at least with the template keyboard: when launching the keyboard in landscape mode it shows the wrong keys, and the 'shift' and 'return' keys are swapped.

IMG_0103

@snomos can you test to see if this happens with the dev build? I've been unable to get my account set up to download via TestFlight even after a few hours of account shuffling, deletion and re-inviting 🥲

snomos commented 11 months ago

Build 0.2.6 (127) of Divvun Dev Keyboards has a preliminary fix for the height issue. @snomos can you test?

The height issue seems to have been fixed. If anything, the keys now seem lower than on Apple's keyboards, but that can be related to the next point:

I think I discovered a new bug as well, at least with the template keyboard: when launching the keyboard in landscape mode it shows the wrong keys, and the 'shift' and 'return' keys are swapped.

This is part of #208, or at least one manifestation of it. What you see is basically the 9" keyboard in landscape mode, and the 12" one in portrait mode. At least that used to be the case on my iPad. Now I ONLY get the 9" keyboard layout for Northern Sami. I changed the layouts for 9" and 12" a bit, so now the only easily seen difference is that the 12" layout has a single quote key to the left of the Á key, in the upper left corner of the keyboard, whereas the 9" layout does NOT have this key - the upper leftmost key is Á on the 9" layout.

Hm, I forgot that the 12" keyboard also has a fourth row of digits, atop the á š e r t y row. So it is easy to see which keyboard is active 🙂

Do you get the same results, @dylanhand ?

snomos commented 11 months ago

On a 9" iPad, I first got the updated keyboard - key rows are roughly the same height as on Apple's keyboards (but each key is larger, with smaller space between each key).

Upon switching from portrait to landscape and back again, the layout switched to the 12" layout. So something is seriously wrong with detecting iPad models.

dylanhand commented 11 months ago

I’ll have a look when I’m back next week:)

On Sep 27, 2023 at 5:59:57 PM, Sjur N Moshagen @.***> wrote:

On a 9" iPad, I first got the updated keyboard - key rows are roughly the same height as on Apple's keyboards (but each key is larger, with smaller space between each key).

Upon switching from portrait to landscape and back again, the layout switched to the 12" layout. So something is seriously wrong with detecting iPad models.

— Reply to this email directly, view it on GitHub https://github.com/divvun/giellakbd-ios/issues/222#issuecomment-1737679696, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADJLKKSKS7DMQKHWARI6ZDX4REP3ANCNFSM6AAAAAA44T6T4Y . You are receiving this because you were mentioned.Message ID: @.***>

dylanhand commented 11 months ago

Hi @snomos

I just set up a small fleet of iPads for testing. So far I'm not getting the same result. Here are the models I'm using:

I'm running build 0.2.6 (127) of Divvun Dev Keyboards with the following results:

Can you confirm you still have this issue and provide steps to reproduce if so? Also please let me know what devices you're using to test.

dylanhand commented 11 months ago

After some more experimentation I see that the single quote character was added to the top left of the Divvun-tastaturer app but evidently not Divvun Dev Keyboards. Will try to push a new build of that app with the height fix and see if it helps.

dylanhand commented 11 months ago

Also @snomos - is there a way I can access the latest layout for Northern Sami? It would help if I can get the layout definition so I can better test locally without having to trigger builds.

In the Template Keyboard app I have a KeyboardDefinitions.json file but didn't see something similar in the lang-sme repo

snomos commented 11 months ago

Hello @dylanhand

My testing devices are:

lang-sme is not the repository for keyboard layouts, the correct repository is keyboard-sme. The layout file for iPads is se.yaml.

The single quote character has been removed from the 9" layout, but that change has not yet been published for the Divvun Keyboards app - all testing and development happens using the Divvun Dev Keyboard, and only when we see that everything is fine, we update the Divvun Keyboards app.

I will update both iPads with the newest version of Divvun Dev Keyboards, and test again.

snomos commented 11 months ago

Here's a screen recording of the 12" iPad. I am not able to trigger switching of layout, but it clearly shows that it is the 9" layout that is used, ie the wrong keyboard layout (from the Divvun Dev Keyboards app):

https://github.com/divvun/giellakbd-ios/assets/1255285/4f61f545-82b2-4c80-9e9b-450180e760c1

snomos commented 11 months ago

On the 9" iPad I am not able to reproduce the issue anymore. I get the correct 9" layout all the time, using the SME keyboard in Divvun Dev Keyboards.

dylanhand commented 11 months ago

@snomos I think I found the size detection issue. It looks like it was having trouble detecting 12.9" iPads when in landscape mode. I just pushed a fix and triggered a build that should be available to test soon.

snomos commented 11 months ago

Excellent! will test when it is ready 🙂

dylanhand commented 11 months ago

Just installed the new build and can confirm it's detecting 12" iPads in landscape mode.

Let me know how it goes for you @snomos :)

snomos commented 11 months ago

With the latest build (128):

https://github.com/divvun/giellakbd-ios/assets/1255285/a8819fc9-1f10-4cf5-847f-1c404be310dc

dylanhand commented 11 months ago

😭 looking into it

dylanhand commented 11 months ago

I see now that the issue causing the 12" keyboard to appear on 9" devices in landscape mode existed in build (123) and (127). I Assume it's there in builds in between as well.

Just so we know this isn't a regression - rather more evidence that the device detection needs a bit more of an overhaul.

snomos commented 11 months ago

Ok, thanks for the update. And isse #208 is evidence there has been problems with device detection for some time.

dylanhand commented 11 months ago

A new build is on its way that may fix the landscape layout issue and issue #208. It seems to be working on the iPads I'm testing with, but should be tested with iPhones as well just to make sure of no regressions.

Please let me know how it works for you @snomos. I'll test with other devices tomorrow.

snomos commented 11 months ago

Build 129 seems to work fine on all three devices I am testing on: iPhone XS, 9" iPad and 12" iPad. I have seen no issues and glitches so far. There are still various discrepancies between keyboard height, key size and font size compared to the Apple keyboards, but layout-wise all three devices serve the layout they should, and nothing else.

dylanhand commented 11 months ago

Great news. I'll do a little more testing on the devices I have here and then move on to the key and font size issues.

snomos commented 11 months ago

As far as I can tell most of the tasks are now properly solved. Everything looks great.

The one thing that is not yet working is the tab key, cf the screen shot:

image

The keyboard is SJD (keyboard-sjd). Two issues:

dylanhand commented 11 months ago

I think it may be required to specify the tab key title in the keyboard definition, similar to how 'return' and 'space' are handled. I can experiment with it tomorrow morning. When I was testing it, it was just titled "tab".

Will also look into background color.

dylanhand commented 11 months ago

And for the sake of documentation, here are the notes I private messaged you earlier today about my changes:

A few notes:

  • I made the keyboard the exact same height as Apple's when in landscape. One difference is that Apple is able to put their spelling suggestions to the right of the undo/redo/copy buttons. We don't have that luxury, so we have to put it below which eats up some pixels. This means that in practice each of our keys is slightly shorter than Apple's
  • Apple's keyboard uses a different height for number keys, which also display those keys in a smaller font. This was out of scope for these changes but is something we could look into if desired. The drawback is that our number keys, since they also contain a swipe-down alt key, are pretty cluttered. I picked a font that was similar to Apple's in size and weight, but that didn't cause overlap for these keys. It would be nice if the font here was at least smaller but that's a separate issue :smile:
snomos commented 11 months ago

I think it may be required to specify the tab key title in the keyboard definition, similar to how 'return' and 'space' are handled. I can experiment with it tomorrow morning. When I was testing it, it was just titled "tab".

Since the tab key is only visible on the large iPad layout, and Apple is using a symbol rather than text, I think we'll do fine with a symbol as well. Something like or similar.

snomos commented 11 months ago

It seems the real issue is that I used the wrong string. it should be tab, not tabs, cf https://github.com/divvun/giellakbd-ios/blob/21a4f7b7c5c708d32562b22cceb45ed23b189feb/Keyboard/Views/KeyView.swift#L298

snomos commented 11 months ago

yes, that was it. After changing from tabs to tab in the layout specification, the tab key works:

image

The only thing left is to fix the key label, to replace the text with a symbol.

snomos commented 11 months ago

I think that on the 12" iPad we should use a symbol also for the return key, and use the text label only on the iPhone and the 9" iPad. That is both in line with Apple's use and fits better the available space. Alternatively, a test for the key with could dynamically determine whether the text fits, and if not use a symbol.

This is also a question of whether one wants a more ISO like key layout, like for SJD above, or a more ANSI style layout, as seen in most of the other Sami Keyboards right now. I would rather switch to the ISO style, as that fits better with what the majority keyboards look like, as well as is more in line with the physical keyboards people are used to.

And with an ISO layout, there is very little space for text strings.

dylanhand commented 11 months ago

Looking into adding a symbol for the tab and return key now.

dylanhand commented 11 months ago

New build in progress that uses an icon for the tab key and shows a return key icon on large iPads.

snomos commented 11 months ago

Nice! Thanks!

snomos commented 11 months ago

TAB key done, seems excellent!

There are other things in the 12" iPad keyboard that is not yet following Apple's layout, I will list them here for documentation, but I don't think we should spend more time on this now. They are:

I will keep this issue open a while still for further discussion of the above points, but otherwise it looks ready to be closed.

dylanhand commented 11 months ago

Great!

I agree those seem like nice things to add. Perhaps worth adding to a separate issue so this one can be closed?

snomos commented 11 months ago

I'll add one final thing since it's directly related to the work done in this issue: the color of the return and tab symbols in dark isn't very user friendly, cf screenshot:

image

dylanhand commented 11 months ago

Oof I forgot to check in dark mode. My apologies. Should be an easy fix - will do tomorrow.

dylanhand commented 11 months ago

Just did a theoretical fix and pushed a build (don't have an iPad Pro atm to test with and the simulator is giving me trouble). I'm pretty sure it will work but will double check it in the morning :)

snomos commented 11 months ago

It works!

dylanhand commented 11 months ago

Excellent :)

snomos commented 11 months ago

I agree those seem like nice things to add. Perhaps worth adding to a separate issue so this one can be closed?

I created #223 for those items.

With the dark mode fix done, I consider all tasks finished, and this issue can be closed.