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

Extend FR and ES keyboards from the EN keyboard #4906

Merged
merged 3 commits into from Apr 12, 2019

Conversation

Projects
None yet
2 participants
@poire-z
Copy link
Contributor

commented Apr 12, 2019

Avoid duplication with the EN keyboard, as most is the same, and just tweak what has to be changed.
Will allow staying in sync with the EN keyboard recent and futur developments of key popups.

Pinging @pazos (#4244) for info, about the ES keyboard: i updated it because there were so little differences from the EN one - it should be the same as before, but it will be easier to tweak if you feel like removing "espacio" (like I did for the FR one) or re-order the key popups (like I did for the FR ones).

The other keyboard layouts are quite a bit more complicated, but some only have changes in the 3,4,5,6th layouts, so they could probably be tweaked with the same kind of remove/insert/replace tweaks if their users feel like enjoying the new popups.

Extend FR and ES keyboards from the EN keyboard
Avoid duplication with the EN keyboard, as most is the
same, and just tweak what has to be changed.
Will allow staying in sync with the EN keyboard recent
and futur developments of key popups.
@Frenzie

This comment has been minimized.

Copy link
Member

commented Apr 12, 2019

It does make it harder to grasp what's going on. I thought you wanted to explicitly reuse the popup variables rather than the whole keyboard. I suppose it shouldn't matter too much. People who want to customize can just base it on the English one.

Incidentally, shouldn't the French layout look more like this? I've never looked at the French keyboard before, but the description of AZERTY as QWERTY with four characters swapped struck me as slightly odd. ;-)

Screenshot_2019-04-12_15-14-29

@poire-z

This comment has been minimized.

Copy link
Contributor Author

commented Apr 12, 2019

It does make it harder to grasp what's going on.

I don't think so. The stuff that changes and why is clearly stated in the comments.
A lot easier, vs the time I spent comparing them to the old std.lua.

I thought you wanted to explicitly reuse the popup variables rather than the whole keyboard.

I thought I would have to do that, and to update it each time you add a popup for a key.
With this, I won't have to (unless some stuff really needs to be adapted).

Incidentally, shouldn't the French layout look more like this?

Yes, M is just right of the L, but it looks like this has never bothered me :) The real pain is AZQW.
And moving the M would require a bit of thinking about what to do with the punctuation and the other layouts, and the popups. So, good enough as it is.

@poire-z

This comment has been minimized.

Copy link
Contributor Author

commented Apr 12, 2019

And moving the M would require a bit of thinking about what to do with the punctuation and the other layouts, and the popups.

(ok, bit of thinking happening right now)

@poire-z

This comment has been minimized.

Copy link
Contributor Author

commented Apr 12, 2019

OK, it will now display as on your screenshot, with some possibly useful chars on the new key in the other modes (except for the russian-like modes where I had no inspiration).

At least, what I thought would be tedious was actually easy with this extend/move/update way of patching the EN keyboard :)

}
for _, popup in ipairs(popups) do
if popup.east and popup.north then -- not all implemented yet

This comment has been minimized.

Copy link
@Frenzie

Frenzie Apr 12, 2019

Member

We can just add a couple of stubs instead? It's the other options that require thought but the basic òöóô-type ones should be fine.

This comment has been minimized.

Copy link
@Frenzie

Frenzie Apr 12, 2019

Member

PS I'll do that now. Shouldn't take more than a few mins.

This comment has been minimized.

Copy link
@poire-z

poire-z Apr 12, 2019

Author Contributor

Dunno (yet) what you mean by stub, but I'll see. But may be no need to add anything.

Anyway, I planned to remove that once you have implemented them for all.
So, no real need for anything more.

(And anyway, it's not even needed (and a bit wrong with my intent) : local toto = "zzz" ; toto.north returns nil, when I thought it would crash with "attempt to index a string" - but no - and if it's not a table it's a no-op)

This comment has been minimized.

Copy link
@Frenzie

Frenzie Apr 12, 2019

Member

Alright, so let's take out the if condition and merge it. ;-)

This comment has been minimized.

Copy link
@poire-z

poire-z Apr 12, 2019

Author Contributor

Alright. (so, UIO are done :)

This comment has been minimized.

Copy link
@Frenzie

Frenzie Apr 12, 2019

Member

Just a stub; it'd require some more research to call it done. :-)

Frenzie added a commit to Frenzie/koreader that referenced this pull request Apr 12, 2019

[UX] Keyboard popups: stubs for the rest of the vowels (I, O, U)
With the basic pattern of diacritics and special characters established by now, the basic diacritics + IPA don't need too much thought for an initial draft.

Cf. koreader#4906 (comment)

Frenzie added a commit that referenced this pull request Apr 12, 2019

[UX] Keyboard popups: stubs for the rest of the vowels (I, O, U) (#4907)
With the basic pattern of diacritics and special characters established by now, the basic diacritics + IPA don't need too much thought for an initial draft.

Cf. #4906 (comment)

@poire-z poire-z merged commit d3aaff1 into koreader:master Apr 12, 2019

1 check passed

ci/circleci Your tests passed on CircleCI!
Details

@poire-z poire-z deleted the poire-z:fr_es_keyboard branch Apr 12, 2019

@Frenzie Frenzie added this to the 2019.05 milestone Apr 19, 2019

@Frenzie Frenzie added the UX label Apr 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.