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.

Sign up for GitHub

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

Jump to bottom

Ability to reorder popup characters. #1921

Open
wants to merge 22 commits into
base: main
Choose a base branch
from
Open

Ability to reorder popup characters. #1921

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
0a5846e
Applying ideas discussed on AnySoftKeyboard/LanguagePack#231
ArenaL5 Dec 15, 2019
1262d7d
Add provisional setting for configuring popup characters
ArenaL5 Dec 18, 2019
674e1ac
Fix for ant:checkstyle
ArenaL5 Dec 19, 2019
1085df1
Fix trivial assertion errors
ArenaL5 Dec 19, 2019
e25917c
Keys without keycodes still have the right to have popupCharacters
ArenaL5 Dec 19, 2019
83c3106
Flush keyboard cache upon change in preferences
ArenaL5 Mar 29, 2020
d097b8b
Update changelog
ArenaL5 Mar 29, 2020
8c71e57
Change layout of popup keys
ArenaL5 Apr 3, 2020
1afbe87
Fix cursor bug
ArenaL5 Jun 22, 2020
9292fcc
Fix AnyKeyboardViewWithMiniKeyboardTest.java
ArenaL5 Apr 25, 2020
254add5
Allow importing unordered layout and reordering single characters
ArenaL5 Apr 20, 2020
c3f9807
Add tests for character reordering
ArenaL5 Apr 28, 2020
380af81
Add better instructions for the characters reordering
ArenaL5 May 11, 2020
d44cebc
Fix test suite
ArenaL5 Jun 21, 2020
c16f2d5
private final String mPopupCharactersOrder
ArenaL5 Jul 14, 2020
3e71793
Remove extraneous execution flag
ArenaL5 Jan 7, 2021
ece84f8
Reminder not to change values-en/strings.xml
ArenaL5 Jan 7, 2021
b728ef7
Reminder to rephrase comment in AnyKeyboard.java
ArenaL5 Jan 11, 2021
ceb9748
Remove complexity
ArenaL5 Jan 11, 2021
ad47846
Coverage for ime/app/src/main/java/com/anysoftkeyboard/ime/AnySoftKey…
ArenaL5 Jan 11, 2021
aab7401
Prevent emoji popups from being mirrored
ArenaL5 Jan 12, 2021
a0adb61
Re-trigger Github Actions
ArenaL5 Jan 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions api/src/main/res/values/keyboard_layout_api.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ Copyright (C) 2013 Menny Even-Danan
<attr name="showPreview"/>
<!-- Whether to auto capitalize. -->
<attr name="autoCap"/>
<!-- Whether the keyboard should be laid out right-to-left (especially useful for pop-ups) -->
<attr name="reverse">
<enum name="auto" value="0" />
<enum name="always" value="1" />
<enum name="never" value="2" />
</attr>
</declare-styleable>

<declare-styleable name="KeyboardLayout_Row">
Expand Down
7 changes: 5 additions & 2 deletions ime/addons/src/test/java/com/anysoftkeyboard/addons/SupportTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,12 @@ public void testDifferentPackageDifferentValues() {

Mockito.verify(remoteRes).getIdentifier("showPreview", "attr", "com.some.other.package");
Mockito.verify(remoteRes).getIdentifier("autoCap", "attr", "com.some.other.package");
Mockito.verify(remoteRes).getIdentifier("reverse", "attr", "com.some.other.package");
Mockito.verifyNoMoreInteractions(remoteRes);

Assert.assertNotSame(backwardCompatibleStyleable, R.styleable.KeyboardLayout);
Assert.assertEquals(backwardCompatibleStyleable.length, R.styleable.KeyboardLayout.length);
Assert.assertEquals(
backwardCompatibleStyleable.length, R.styleable.KeyboardLayout.length - 1);
Assert.assertEquals(backwardCompatibleStyleable.length, sparseIntArray.size());
for (int attrId : backwardCompatibleStyleable) {
if (attrId == 123) {
Expand Down Expand Up @@ -109,11 +111,12 @@ public void testDifferentPackageNoValue() {

Mockito.verify(remoteRes).getIdentifier("showPreview", "attr", "com.some.other.package");
Mockito.verify(remoteRes).getIdentifier("autoCap", "attr", "com.some.other.package");
Mockito.verify(remoteRes).getIdentifier("reverse", "attr", "com.some.other.package");
Mockito.verifyNoMoreInteractions(remoteRes);

Assert.assertNotSame(backwardCompatibleStyleable, R.styleable.KeyboardLayout);
Assert.assertEquals(
backwardCompatibleStyleable.length, R.styleable.KeyboardLayout.length - 2);
backwardCompatibleStyleable.length, R.styleable.KeyboardLayout.length - 3);
Assert.assertEquals(backwardCompatibleStyleable.length, sparseIntArray.size());
for (int attrId : backwardCompatibleStyleable) {
Assert.assertEquals(attrId, sparseIntArray.get(attrId));
Expand Down
2 changes: 2 additions & 0 deletions ime/app/src/main/java/com/anysoftkeyboard/ime/AnySoftKeyboardRxPrefs.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,8 @@ protected void onSharedPreferenceChange(String key) {
|| key.startsWith(KeyboardExtensionFactory.BOTTOM_ROW_PREF_ID_PREFIX)
|| key.startsWith(KeyboardExtensionFactory.TOP_ROW_PREF_ID_PREFIX)) {
onAddOnsCriticalChange();
} else if (key.equals("settings_key_popup_characters_order")) {
onAddOnsCriticalChange();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any chance to add a unit-test for this?
Basically, change this setting value and see that the cached keyboards were re-created.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use the R.string.settings_key_popup_characters_order in the test. It is fine to use this string here in the keyboard code, but in tests we should use the value from the resources.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this enough? ad47846

}
}

Expand Down
9 changes: 2 additions & 7 deletions ime/app/src/main/java/com/anysoftkeyboard/keyboards/AnyKeyboard.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -675,13 +675,8 @@ protected boolean setupKeyAfterCreation(AnyKey key) {
return true;
}

// filling popup res for external keyboards
if (key.popupCharacters != null) {
if (key.popupCharacters.length() > 0) {
key.popupResId = com.menny.android.anysoftkeyboard.R.xml.popup_one_row;
}
return true;
}
// if only popupCharacters are specified, call super nevertheless --
// we might want to add more characters.
ArenaL5 marked this conversation as resolved.
Show resolved Hide resolved

return false;
}
Expand Down
32 changes: 10 additions & 22 deletions ime/app/src/main/java/com/anysoftkeyboard/keyboards/AnyPopupKeyboard.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,20 +62,11 @@ public AnyPopupKeyboard(
loadKeyboard(keyboardDimens);

final int rowsCount = getPopupRowsCount(popupCharacters);
final int popupCharactersLength =
Character.codePointCount(popupCharacters, 0, popupCharacters.length());
final int keysPerRow = (int) Math.ceil((float) popupCharactersLength / (float) rowsCount);

List<Key> keys = getKeys();
for (int rowIndex = rowsCount - 1; rowIndex >= 0; rowIndex--) {
int baseKeyIndex = keys.size() - rowIndex - 1;
addPopupKeysToList(
baseKeyIndex,
keyboardDimens,
keys,
popupCharacters,
rowIndex * keysPerRow,
keysPerRow);
addPopupKeysToList(baseKeyIndex, keyboardDimens, keys, popupCharacters, rowIndex);
}
}

Expand All @@ -84,8 +75,8 @@ private void addPopupKeysToList(
KeyboardDimens keyboardDimens,
List<Key> keys,
CharSequence popupCharacters,
int characterOffset,
int keysPerRow) {
int characterOffset) {
final int rowsCount = getPopupRowsCount(popupCharacters);
int rowWidth = 0;
AnyKey baseKey = (AnyKey) keys.get(baseKeyIndex);
Row row = baseKey.row;
Expand All @@ -104,10 +95,9 @@ private void addPopupKeysToList(
AnyKey aKey = null;
final int popupCharactersLength =
Character.codePointCount(popupCharacters, 0, popupCharacters.length());
for (int popupCharIndex = characterOffset + 1;
popupCharIndex < characterOffset + keysPerRow
&& popupCharIndex < popupCharactersLength;
popupCharIndex++) {
for (int popupCharIndex = characterOffset + rowsCount;
popupCharIndex < popupCharactersLength;
popupCharIndex += rowsCount) {
x += (keyHorizontalGap / 2);

aKey = new AnyKey(row, keyboardDimens);
Expand Down Expand Up @@ -157,12 +147,10 @@ private static int getPopupLayout(CharSequence popupCharacters) {

private static int getPopupRowsCount(CharSequence popupCharacters) {
final int count = Character.codePointCount(popupCharacters, 0, popupCharacters.length());
if (count <= 8) return 1;
if (count <= 16) {
return 2;
} else {
return 3;
}

if (count <= 3) return 1;
if (count <= 12) return 2;
return 3;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's strange that we do not have a unit-test for this.
Maybe in the allAddons variant?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

testCodesParsing() at ExternalAnyKeyboardTest.java tests it indirectly.

I could add a test-unit to test this directly, but how do I test a private method?

}

@Override
Expand Down