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

Adding a new line in auto completion view when the width of candidates is too big. #1173

Merged
merged 1 commit into from
May 9, 2019

Conversation

daoshengmu
Copy link
Contributor

No description provided.

@daoshengmu
Copy link
Contributor Author

Screen Shot 2019-05-06 at 11 19 22 AM

We will see this issue when using Zhuyin input (#1161)

We are trying to fix this problem, and this can be reproduced when typing ㄉㄚˋ or ㄏㄠˇ with Zhuyin keyboard.

@@ -60,6 +62,8 @@ public AutoCompletionView(Context aContext, AttributeSet aAttrs, int aDefStyle)
private void initialize(Context aContext) {
inflate(aContext, R.layout.autocompletion_bar, this);
mFirstLine = findViewById(R.id.autoCompletionFirstLine);
mLineWidth = WidgetPlacement.dpDimension(getContext(), R.dimen.keyboard_width);
mPerKeyWidth = WidgetPlacement.dpDimension(getContext(), R.dimen.keyboard_key_width);
Copy link
Contributor Author

@daoshengmu daoshengmu May 7, 2019

Choose a reason for hiding this comment

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

I am trying to get the correct line width from mFirstLine, but it always returns zero from getWidth() or getMeasuredWidth(). R.dimen.keyboard_width is not exactly correct because our real width should be the same with world_width.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MortimerGoro I am trying to use post runnable to make sure this UI has been loaded. but it still shows zero.

       mFirstLine.post(() -> {
            mLineWidth = mFirstLine.getWidth(); // the width is still not ready...
        });

Alternatively, R.dimen.world_width is more close to the correct result.

@@ -60,6 +62,8 @@ public AutoCompletionView(Context aContext, AttributeSet aAttrs, int aDefStyle)
private void initialize(Context aContext) {
inflate(aContext, R.layout.autocompletion_bar, this);
mFirstLine = findViewById(R.id.autoCompletionFirstLine);
mLineWidth = WidgetPlacement.pixelDimension(getContext(), R.dimen.world_width);
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not correct to use the world width here. Ideally the View should get the current view width and detect layout changes. You can override onSizeChanged in order to detect the size and layout of the whole view: https://developer.android.com/reference/android/view/View.html#onSizeChanged%28int,%20int,%20int,%20int%29

@bluemarvin
Copy link
Contributor

I'll wait for the review comment to be addressed before landing this one.

@daoshengmu daoshengmu force-pushed the wrongAutoCompletionAlignment branch 2 times, most recently from 29bdac0 to abb3dcf Compare May 8, 2019 23:18
@cvan cvan mentioned this pull request May 9, 2019
@daoshengmu daoshengmu force-pushed the wrongAutoCompletionAlignment branch from abb3dcf to 5d1af4e Compare May 9, 2019 00:29
@daoshengmu
Copy link
Contributor Author

I decide to add <dimen name="autocompletion_widget_line_width">530dp</dimen>, it will make things simple.

@daoshengmu daoshengmu self-assigned this May 9, 2019
@cvan cvan added this to the v1.2 milestone May 9, 2019
@@ -103,6 +103,7 @@
<dimen name="no_internet_z_distance" format="float" type="dimen">2.5</dimen>

<!-- Autocompletion Widget -->
<dimen name="autocompletion_widget_line_width">530dp</dimen>
Copy link
Contributor

Choose a reason for hiding this comment

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

Some keyboards will have a different width so this value won't match and the ideal thing is detect onResize changes. Anyway it will work for chinese keyboard for now, as it size is fixed and only them have the autocompletion. We can implement the better solution in a follow-up, please create a issue for it

@MortimerGoro MortimerGoro merged commit 13808e7 into master May 9, 2019
@cvan cvan deleted the wrongAutoCompletionAlignment branch May 9, 2019 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants