search

stanipintjuk / Silverfish

A simple and lightweight launcher for Android.
GNU General Public License v3.0
44 stars 17 forks source link

Mods to fix issue #53 #69

Closed MPArnold closed 6 years ago

MPArnold commented 6 years ago

TabbedAppDrawerFragment.java now has code that moves a dragged item to the home page once it reaches the far right hand side of the screen. (The exact threshold being determined by a new method in common.Utils)

If the icon is dragged minimally the pop-up menu is triggered allowing user to achieve the same thing.

Because I run (Win10) Android Studio 3 I was forced to rewrite common.Utils.loadAppIconAsync() to be fully static. (Github would not accept otherwise) See the stackoverflow question documented in the code.

Finally I added a new resource to give a larger Icon size on tablets.

I apologize for the apparent volume of changes - for some reason AS took it upon itself to apply a lot of minor formatting changes!!!!

Have tested on Samsung Galaxy S3 handheld and Samsung Galaxy Tab S2 tablet.

Lonami commented 6 years ago

Cool, if the code still works but also works for you now we can merge. Let me test.

Lonami commented 6 years ago

Sorry but these fixes aren't working at all on my Xperia M4 (Marshmallow, 6.0) device!

Lonami commented 6 years ago

@uraza thanks for the review :P yes he could run the autoformat on his changes.

MPArnold commented 6 years ago

I also queried the nicety of an absolute threshold. It works fine on my Samsung devices because OS suspends drag at very edge of screen and passes the address of the RH pixel to the DROP event. Maybe not so good on other devices but I really don't know! Will change the constant to a float representing 95% of the screen width to give better device independence. Have attended to your formatting comments also.

On Fri, Feb 9, 2018 at 2:53 AM, Uraza notifications@github.com wrote:

@uraza commented on this pull request.

In app/src/main/java/com/launcher/silverfish/launcher/appdrawer/ TabbedAppDrawerFragment.java https://github.com/stanipintjuk/Silverfish/pull/69#discussion_r166934868 :

  • // getX() and getY() now return relative offsets,
  • // so accumulate them to get the total movement
  • dragOffsetX += dragEvent.getX();
  • dragOffsetY += dragEvent.getY();
  • // If drag is on the way out of this page then stop receiving drag events
  • int threshold = Constants.SCREEN_CORNER_THRESHOLD;
  • // Get display size
  • int screen_width = Utils.getScreenDimensions(getActivity()).x;
  • if (dragEvent.getX() > screen_width - threshold) {
  • return false;
  • } else {
  • // Do nothing if inside 'Move to Home Page' zone
  • if (!Utils.isBeyondRightHandThreshold(getActivity(),dragEvent)) {

Missing space after comma.

In app/src/main/java/com/launcher/silverfish/launcher/appdrawer/ TabbedAppDrawerFragment.java https://github.com/stanipintjuk/Silverfish/pull/69#discussion_r166934968 :

                         launchUninstallIntent(appName);
  • } else if (Utils.isBeyondRightHandThreshold(getActivity(),dragEvent)) {

Same as above.

In app/src/main/java/com/launcher/silverfish/launcher/appdrawer/ TabbedAppDrawerFragment.java https://github.com/stanipintjuk/Silverfish/pull/69#discussion_r166938478 :

@@ -313,15 +300,22 @@ public boolean onDrag(View view, DragEvent dragEvent) { }

                 case DragEvent.ACTION_DROP: {

+

  • dragOffsetX = dragBeginX-dragEvent.getX(); // Total x movement

Should add a space around the - operator to be consistent with the coding style adopted in your Utils.java changes.

In app/src/main/java/com/launcher/silverfish/common/Utils.java https://github.com/stanipintjuk/Silverfish/pull/69#discussion_r166938737 :

/**

  • Created by Stanislav Pintjuk on 8/3/16.
  • E-mail: stanislav.pintjuk@gmail.com */ public class Utils {
  • /**
    • Is supplied drag event within the 'move to home page' zone?
  • */
  • public static boolean isBeyondRightHandThreshold(Activity a, DragEvent dragEvent) {
  • int xThreshold = Utils.getScreenDimensions(a).x - Constants.SCREEN_CORNER_THRESHOLD;

Is that SCREEN_CORNER_THRESHOLD value sufficiently high in practice? Just wondering if there were not too much failed attempts to drag the icon out of the screen when you tried it. :)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/stanipintjuk/Silverfish/pull/69#pullrequestreview-95073859, or mute the thread https://github.com/notifications/unsubscribe-auth/AKpl6SVf8DxyHKL--A0wHFCD8dGDpYV0ks5tSvxRgaJpZM4R-F43 .

MPArnold commented 6 years ago

So... Works on my handheld (Jelly Bean 4.3) Fails on your Xperia (Marshmallow 6.0) Works on my tablet (Nougat 7.1) Fascinating! Must get to the bottom of this problem.

Possible explanation: Simultaneous triggering of DragListeners in both activity and fragment Maybe this is a conflict? Is this behaviour expected and/or desirable? - Your thoughts would be appreciated.

Meanwhile, will add a simple debug mode to this app and investigate further...

On Fri, Feb 9, 2018 at 2:31 AM, Lonami notifications@github.com wrote:

Sorry but these fixes aren't working at all on my Xperia M4 (Marshmallow, 6.0) device!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/stanipintjuk/Silverfish/pull/69#issuecomment-364111817, or mute the thread https://github.com/notifications/unsubscribe-auth/AKpl6axJK457iW55hq1owdOBmcgT3SHwks5tSvcxgaJpZM4R-F43 .

MPArnold commented 6 years ago

Silverfish Version 0.5.3 (9) [Proposed]

In simple terms, the F-droid distribution of Version 0.5.2 (8) is bad. [eg: issue #53]

Initial attempts to deal with #53 did not succeed on some devices.

I have extensively clarified and documented code in the main activity and primary page segments. I suggest you review them in their new entirety rather than by picking your way through the changes. [Changes to the effective logic are actually relatively minor, but debugging will become simpler.]

I have NOT committed my local gradle changes [As noted in (a) below.] So have a play with the current set of commits. If there are still problems consider retrying after making the suggested gradle changes yourself or, alternatively, download my APK

Feel free to contact me here: koparapara@gmail.com

Version 9 highlights:

(a) Gradle scripts targeting gradle 4.1, build tools 27.0.3, appcompat-v7 25.4.0 ( Developers building privately will require Android Studio 3. ) NB: targetSdkVersion 25 remains unchanged.

(b) Serviceability enhancements:

(c) Device-independent threshold calculations for 'drag' events. (see Utils)

(d) Standardisation of 'pop-up' menu following 'minimal' drag/drop).

(e) DragListener logic tidy-up.

(f) Larger Icon size for tablets.

MPArnold commented 6 years ago

Oops, didn't mean to close it!

Lonami commented 6 years ago

You're putting a lot of effort into this, and I appreciate. I tested your changes and now it seems to work (I can drop without moving and the menu shows, and so does dropping on the right side). I noticed I have to drop on the right side which then puts the item in the main screen, but it doesn't move to the main screen until I drop which is probably what confused me before. I guess some feedback there (like moving to the screen as it was before) would be nice. Besides that it works.

Regarding your logging class… I can't see why manually rewrite something like that. It doesn't even provide logging to file, and logging at all shouldn't be something a normal user has to concern about. Debug logging, info, warning and error are already builtin settings and you can choose whether to display them or not without manually checking flags like you are doing. I don't think we need that developer section category and it's not related at all to #53!

Again, formatting could be better although code is fine, besides the comments I left (mostly, hardcoding things like colours). It would be nice if @stanipintjuk could get some time to check this out but well, one can't always find time for this :P

So in all I'd run the default code formatter on the files you've modified (right click from Android Studio and reformat?) and possible get rid of the logger and developer options (while you put effort into them, they don't belong here).

About the fact that dragging to the side doesn't change the screen until I drop, is that a feature or a bug?

MPArnold commented 6 years ago

I fear this exchange could go on ad infinitum.

Lonami commented 6 years ago

Took me a while to nitpick all the relevant changes… All I told you was that the log class was unnecessary and that the style wasn't at its best. I would've pushed changes to your branch if you hadn't deleted it, but oh well. Still mentioned you're the author of such changes. Thanks!