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

Suggestion strip incognito icon cut off on top and bottom #221

Closed
syphyr opened this issue Oct 17, 2023 · 12 comments
Closed

Suggestion strip incognito icon cut off on top and bottom #221

syphyr opened this issue Oct 17, 2023 · 12 comments
Labels
bug Something isn't working

Comments

@syphyr
Copy link
Contributor

syphyr commented Oct 17, 2023

Does it happen in OpenBoard, or is it exclusive to this modified version?
This occurs in unmodified openboard app

Describe the bug
The top and bottom of the circle behind the incognito icon in suggestion strip is cut off

To Reproduce
Steps to reproduce the behavior:

  1. Open suggestion strip in incognito mode
  2. See icon is cut off

Expected behavior
The circle dawn behind the incognito icon should be smaller to fit within suggestion strip

Screenshots
If applicable, add screenshots to help explain your problem.
If you add screenshots, please reduce the size or use thumbnails to keep the issue nicely readable.
Screenshot_incognito

App version
Which exact version of this fork is affected?
v1.4.5_new_v10-20-gec1a91d1

Smartphone (please complete the following information):

  • Device: T813
  • OS: Android 7.1 (cm-14.1)

Additional context
The screen resolution on my T813 tablet is 1536 x 2048 pixels, 4:3 ratio (~264 ppi density)

@syphyr syphyr added the bug Something isn't working label Oct 17, 2023
@syphyr
Copy link
Contributor Author

syphyr commented Oct 17, 2023

A similar issue #191 was recently fixed by 78e924e but that only fixes the left side of the icon.

Adding android:paddingTop and android:paddingBottom to suggestions_strip.xml also does not fix this issue.

@syphyr
Copy link
Contributor Author

syphyr commented Oct 17, 2023

It seems this has to be fixed programmatically. I've tried something like this, but no luck so far:

--- a/app/src/main/java/org/dslul/openboard/inputmethod/latin/suggestions/SuggestionStripView.java
+++ b/app/src/main/java/org/dslul/openboard/inputmethod/latin/suggestions/SuggestionStripView.java
@@ -207,7 +207,11 @@ public final class SuggestionStripView extends RelativeLayout implements OnClick
         mOtherKey.setOnClickListener(this);
         mOtherKey.setImageDrawable(Settings.getInstance().getCurrent().mIncognitoModeEnabled ? mIncognitoIcon : mToolbarArrowIcon);
         mOtherKey.setColorFilter(colors.getKeyText()); // maybe different color?
-        mOtherKey.setBackground(new ShapeDrawable(new OvalShape())); // ShapeDrawable color is black, need src_atop filter
+        ShapeDrawable drawable = new ShapeDrawable(new OvalShape());
+        drawable.setIntrinsicHeight(20);
+        drawable.setIntrinsicWidth(20);
+        drawable.setBounds(0,0,20,20);
+        mOtherKey.setBackground(drawable); // ShapeDrawable color is black, need src_atop filter
         mOtherKey.getBackground().setColorFilter(colors.getDoubleAdjustedBackground(), PorterDuff.Mode.SRC_ATOP);
         mOtherKey.getLayoutParams().height *= 0.82; // shrink the whole key a little (drawable not affected)
         mOtherKey.getLayoutParams().width *= 0.82;

@Helium314
Copy link
Owner

The icon actually seems fine, only the background is the issue.
Setting the width / height to *0.5 instead of *0.82 works on my phone for landscape, but in portrait the background now is too small.

I don't know why Android wants to have the background larger than the actual button...
Setting a shape background in xml doesn't help.

The main issue seems to be that android:maxHeight and android:maxWidth seem to be ignored.
Possibly could be worked around by reading config_suggestions_strip_height and config_suggestions_strip_edge_key_width in code and setting width and height to the minimum of these.

@syphyr
Copy link
Contributor Author

syphyr commented Oct 18, 2023

There are actually two places that the height and width of the background need to be adjusted. One for the mOtherKey and one for the mEnabledToolKeyBackground. I changed the width / height to *0.72 instead of *0.82 and it fits perfectly now. I also changed the divisor for the radius of setGradientRadius to 2.77f (which equals 0.72 for the diameter).
incognito

syphyr@771d8bb

Since the aspect ratio of my device is 4:3, the landscape and portrait sizes are the same.

@syphyr
Copy link
Contributor Author

syphyr commented Oct 18, 2023

I would make a PR for the changes I made above, but I'm not sure how they would look on other devices with different resolutions and aspect ratios.

@BlackyHawky
Copy link
Contributor

I tested your commit and on my Huawei phone (1344px × 2772px), the defect is always present in landscape mode.
I can confirm that the background in portrait mode is too small.

@Helium314
Copy link
Owner

I removed the maxHeight and maxWidth from xml, now it will be determined in code.
Work for me, I hope that's the case for you as well.

@syphyr
Copy link
Contributor Author

syphyr commented Oct 20, 2023

I removed the maxHeight and maxWidth from xml, now it will be determined in code. Work for me, I hope that's the case for you as well.

just FYI, your change fixed the issue and it looks perfect now

@BlackyHawky
Copy link
Contributor

@Helium314
Since your commit fd65e92, a white bar appears only in landscape mode ☹

Watch video
Bug.mp4

All apps are affected.
Tested on Huawei phone with Android 12.

Do you have any ideas?

@Helium314
Copy link
Owner

No idea, and nothing in that commit should be able to summon anything outside the toolbar key.
Which part of the commit causes it to appear? Setting layout parameters, or removing maxWidth/maxHeight?

@BlackyHawky
Copy link
Contributor

No idea, and nothing in that commit should be able to summon anything outside the toolbar key. Which part of the commit causes it to appear? Setting layout parameters, or removing maxWidth/maxHeight?

Sorry, it has nothing to do with this commit.

My phone went from Android 10 to Android 12 yesterday.
I have no idea where this could have come from.
I'm going to test all the debug versions until I find out how long ago this bug occurred...

@BlackyHawky
Copy link
Contributor

I've tested the first release and the bug is there.
So it's certainly due to my phone with Android 12.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants