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

Add support for resizing alphabetic keyboard with based on language #1177

Merged
merged 1 commit into from
May 10, 2019

Conversation

MortimerGoro
Copy link
Contributor

@MortimerGoro MortimerGoro commented May 8, 2019

While implementing the german keyboard I had some width space problems (they have a extra letter in the same line as enter). I tried this as a solution but then I switched to making keys a bit smaller.

I believe @daoshengmu also needs this feature for Zhuyin keybord. It may be required for russian too.

@MortimerGoro MortimerGoro changed the title Ad support for resizing alphabetic keyboard with based on language Add support for resizing alphabetic keyboard with based on language May 8, 2019
Copy link
Contributor

@daoshengmu daoshengmu left a comment

Choose a reason for hiding this comment

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

After running this patch on Oculus Go, I saw the keyboard width is wrong when launching the custom keyboard at the first time. There is a grey layer upon the keyboard when choosing languages, its width is also wrong at the beginning.

// Fall back to english.
mCurrentKeyboard = getKeyboardForLocale(Locale.ENGLISH);
keyboard = getKeyboardForLocale(Locale.GERMAN);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a typo? It looks like it should still be getKeyboardForLocale(Locale.ENGLISH);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch, yes it was a typo (changed it for a test)

@@ -568,10 +579,20 @@ private void handleGlobeClick() {

private void handleLanguageChange(KeyboardInterface aKeyboard) {
mCurrentKeyboard = aKeyboard;
int width = getKeyboardWidth(mCurrentKeyboard.getAlphabeticKeyboardWidth());
Copy link
Contributor

Choose a reason for hiding this comment

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

I would like to add "final" to when it is a constant.

Resources resources = aContext.getResources();
DisplayMetrics metrics = resources.getDisplayMetrics();
float px = dp * ((float)metrics.densityDpi / DisplayMetrics.DENSITY_DEFAULT);
int px = (int) Math.ceil(dp * ((float)metrics.densityDpi / DisplayMetrics.DENSITY_DEFAULT));
return px;
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe just return (int) Math.ceil(dp * ((float)metrics.densityDpi / DisplayMetrics.DENSITY_DEFAULT));

SettingsStore.getInstance(getContext()).setSelectedKeyboard(aKeyboard.getLocale());
mKeyboardView.setKeyboard(mCurrentKeyboard.getAlphabeticKeyboard());
hideOverlays();
updateCandidates();

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: remove this newline.

@cvan cvan added this to the v1.2 milestone May 9, 2019
@philip-lamb philip-lamb added enhancement This issue is a new feature or request UX Issue relates to UX in progress labels May 9, 2019
@MortimerGoro
Copy link
Contributor Author

I fixed all issues, I'm going to merge this because I need it for other PRs. We can test the work together when we land the next keyboard updates

@MortimerGoro MortimerGoro merged commit a1e8c7d into master May 10, 2019
@cvan cvan deleted the keyboard_width branch May 10, 2019 11:46
@daoshengmu
Copy link
Contributor

I just confirmed it works properly on Focus Plus. Good job!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This issue is a new feature or request UX Issue relates to UX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants