Sort menu redesign - Bottom Sheet Dialog #1242

shobhitagarwal1612 · 2017-07-10T04:07:40Z

lognaturel

This looks and works really well for me in portrait! It disappears on screen rotation, though, and doesn't show all the way in landscape. This is on a Galaxy Tab A, Android 5.1.1.

lognaturel · 2017-07-11T21:45:53Z

collect_app/src/main/java/org/odk/collect/android/utilities/ApplicationConstants.java

@@ -28,6 +31,24 @@
            "sq", "sw", "sw_KE", "ta", "th_TH", "tl", "tr", "uk", "ur",
            "ur_PK", "vi", "zh", "zu"};

+    public static int getSortIcon(String sortBy) {


This constants class should really only hold true constants and no logic.

shobhitagarwal1612 · 2017-07-12T20:57:48Z

@lognaturel Please review

lognaturel · 2017-07-14T04:09:31Z

Nice, @shobhitagarwal1612! For Edit Saved Form, there are a lot of options and so some scrolling is needed to see all of them. That wasn't immediately obvious to me. Is there some standard way of identifying that scrolling may be needed?

@mmarciniak90 can you please do a test pass? @grzesiek2010 would be great to get your eyes on it as well and see what you think. I also want to do another code review pass.

I need to do another thorough code review.

mmarciniak90 · 2017-07-19T14:24:43Z

I noticed problems. Tested on Android 4.1, 4.2, 4.4, 5.1.
Use case:

User is on 'Get Blank forms' view
User opens sort dialog
User changes device orientation and backs to vertical orientation
User locks screen
User unlocks screen
User sees that sort dialog is opened second time

Other similar case:

User is on 'Get Blank forms' view
User opens sort dialog
User changes device orientation and backs to vertical orientation
User closes sort dialog
User locks screen
User unlocks screen
Sort dialog appears

Those problems occur only on sort list with 2 options

shobhitagarwal1612 · 2017-07-20T15:07:08Z

@mmarciniak90 @lognaturel Please review

lognaturel · 2017-07-25T04:21:40Z

@grzesiek2010 @mmarciniak90 @yanokwa I think this is really nice and I think it's pretty unlikely to confuse users. What would you think about merging it to master? That way we'd have a solution that better matches current design guidelines and that could stay the same through a more significant UI refresh.

grzesiek2010 · 2017-07-25T09:46:43Z

Samsung Galaxy Duos
Android 4.2.2

07-25 11:13:26.009 15937-15937/org.odk.collect.android E/WindowManager: android.view.WindowLeaked: Activity org.odk.collect.android.activities.InstanceChooserList has leaked window com.android.internal.policy.impl.PhoneWindow$DecorView{c29124d V.E..... R....... 0,0-1920,1005} that was originally added here
at android.view.ViewRootImpl.(ViewRootImpl.java:465)
at android.view.WindowManagerGlobal.addView(WindowManagerGlobal.java:277)
at android.view.WindowManagerImpl.addView(WindowManagerImpl.java:69)
at android.app.Dialog.show(Dialog.java:312)
at org.odk.collect.android.activities.AppListActivity.setupBottomSheet(AppListActivity.java:313)
at org.odk.collect.android.activities.AppListActivity.onResume(AppListActivity.java:136)
at org.odk.collect.android.activities.InstanceChooserList.onResume(InstanceChooserList.java:156)
at android.app.Instrumentation.callActivityOnResume(Instrumentation.java:1255)
at android.app.Activity.performResume(Activity.java:6412)
at android.app.ActivityThread.performResumeActivity(ActivityThread.java:3392)
at android.app.ActivityThread.handleResumeActivity(ActivityThread.java:3434)
at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2772)
at android.app.ActivityThread.handleRelaunchActivity(ActivityThread.java:4471)
at android.app.ActivityThread.access$1000(ActivityThread.java:177)
at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1455)
at android.os.Handler.dispatchMessage(Handler.java:102)
at android.os.Looper.loop(Looper.java:145)
at android.app.ActivityThread.main(ActivityThread.java:5951)
at java.lang.reflect.Method.invoke(Native Method)
at java.lang.reflect.Method.invoke(Method.java:372)
at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1400)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1195)

I see this crash if i open the sort menu and change the device orientation. it works well only in Send Finalized Form and Delete Saved Form

I'm not able to reproduce this issue on Samsung Galaxy S4 Android 5.0.1

grzesiek2010 · 2017-07-25T09:51:09Z

Both icons are blue if i select one of available options (it work well by default after installation):

I noticed this issue also using Samsung Galaxy Duos Android 4.2.2 it looks ok on Samsung Galaxy S4 Android 5.0.1

shobhitagarwal1612 · 2017-07-25T13:54:31Z

@grzesiek2010 Fixed

grzesiek2010 · 2017-07-25T14:26:55Z

ok I'll look at it again.

shobhitagarwal1612 · 2017-07-26T04:53:30Z

@lognaturel I have switched the base branch to master

grzesiek2010 · 2017-07-26T17:52:29Z

The crash doesn't occur but the problem with blue icons is still visible.

shobhitagarwal1612 · 2017-07-26T17:54:14Z

@grzesiek2010 In which screen did you notice this?

grzesiek2010 · 2017-07-26T17:56:25Z

each one

Omits `by` from the sort options labels

shobhitagarwal1612 · 2017-07-27T16:14:53Z

@grzesiek2010 I am unable to reproduce the bug. I am testing on API 25 emulator. Can you please describe the steps to reproduce this bug?

grzesiek2010 · 2017-07-28T10:03:49Z

There aren't any special steps just:

Open any list
Open Sort menu
Click on Name, A-Z or Name, Z-A
Open the menu again and both icons (Name, A-Z or Name, Z-A) are blue.

The same if you use Date options. Everything is ok if you sort by status because there are two different icons I guess.

shobhitagarwal1612 · 2017-07-31T05:44:27Z

@grzesiek2010 Fixed. Please test again.

grzesiek2010 · 2017-07-31T11:22:38Z

@shobhitagarwal1612 @lognaturel I can confirm that both issues I found have been fixed. @mmarciniak90 will continue testing when she is free.

lognaturel · 2017-08-09T15:56:28Z

@shobhitagarwal1612 If you fix these merge conflicts one more time, this is ready to go in. 🎉
@mmarciniak90 can take another pass at it after it's merged and file issues if needed.

# Conflicts: # collect_app/src/main/res/values-hi/strings.xml # collect_app/src/main/res/values-sq/strings.xml # collect_app/src/main/res/values-sv-rSE/strings.xml

…e-1039-sortdialog # Conflicts: # collect_app/src/main/res/values-sv-rSE/strings.xml

shobhitagarwal1612 · 2017-08-09T16:21:50Z

@lognaturel Merged the conflicts

lognaturel previously requested changes Jul 11, 2017

View reviewed changes

lognaturel added the changes requested label Jul 11, 2017

shobhitagarwal1612 force-pushed the issue-1039-sortdialog branch from fc1898c to f2597b3 Compare July 12, 2017 05:07

lognaturel removed the changes requested label Jul 13, 2017

lognaturel added needs testing reviews wanted labels Jul 14, 2017

mmarciniak90 mentioned this pull request Jul 19, 2017

Sort on 'Fill Blank Form' view takes into account a case-sensitive #1264

Closed

lognaturel added changes requested and removed needs testing labels Jul 19, 2017

shobhitagarwal1612 force-pushed the issue-1039-sortdialog branch from 190d37e to db805ea Compare July 20, 2017 05:04

lognaturel added needs testing and removed changes requested labels Jul 20, 2017

shobhitagarwal1612 changed the base branch from ui-refresh to master July 26, 2017 04:05

shobhitagarwal1612 force-pushed the issue-1039-sortdialog branch from 4fe9130 to ddaf9cf Compare July 26, 2017 04:51

shobhitagarwal1612 force-pushed the issue-1039-sortdialog branch from 20d1c3b to 656e0ca Compare July 26, 2017 08:01

Adds icons

7d97d15

shobhitagarwal1612 added 9 commits July 27, 2017 21:31

converts the method to return hashmap in ApplicationConstants

b807c08

Retains the state of the bottom sheet on rotation

8c70808

Adjusts the peek height

b3be9b0

Adjusts the peek height to display all the items in landscape mode

84ecc5b

Omits `by` from the sort options labels

Fixes dialog showing on screen lock/unlocking

ff66b72

Fixes WindowLeaked error while changing orientation

51d3c1f

Fixes the blue icon for previous selected item

c7c4da0

uses tintColor in dialog adapter

0fc4e57

Removes unused color

5165133

shobhitagarwal1612 force-pushed the issue-1039-sortdialog branch from 656e0ca to 5165133 Compare July 27, 2017 16:12

Fixes wrong tint color in pre-lollipop devices

8ab3360

Merge branch 'master' into issue-1039-sortdialog

e05dee4

lognaturel added changes requested and removed needs testing reviews wanted labels Aug 9, 2017

shobhitagarwal1612 added 2 commits August 9, 2017 21:47

Merge branch 'master' into issue-1039-sortdialog

1f861d5

# Conflicts: # collect_app/src/main/res/values-hi/strings.xml # collect_app/src/main/res/values-sq/strings.xml # collect_app/src/main/res/values-sv-rSE/strings.xml

Merge remote-tracking branch 'origin/issue-1039-sortdialog' into issu…

a5b6e58

…e-1039-sortdialog # Conflicts: # collect_app/src/main/res/values-sv-rSE/strings.xml

Removes unused resources

b44521f

lognaturel approved these changes Aug 9, 2017

View reviewed changes

lognaturel merged commit 2f5d5de into getodk:master Aug 9, 2017

grzesiek2010 mentioned this pull request Aug 17, 2017

Refactoring dialogs #541

Closed

snavarreteimmap pushed a commit to snavarreteimmap/collect that referenced this pull request Oct 2, 2020

Use bottom sheet dialog for sort menus (getodk#1242)

d2340b4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sort menu redesign - Bottom Sheet Dialog #1242

Sort menu redesign - Bottom Sheet Dialog #1242

shobhitagarwal1612 commented Jul 10, 2017

lognaturel left a comment

lognaturel Jul 11, 2017

shobhitagarwal1612 commented Jul 12, 2017

lognaturel commented Jul 14, 2017

mmarciniak90 commented Jul 19, 2017

shobhitagarwal1612 commented Jul 20, 2017

lognaturel commented Jul 25, 2017

grzesiek2010 commented Jul 25, 2017

grzesiek2010 commented Jul 25, 2017

shobhitagarwal1612 commented Jul 25, 2017

grzesiek2010 commented Jul 25, 2017

shobhitagarwal1612 commented Jul 26, 2017

grzesiek2010 commented Jul 26, 2017

shobhitagarwal1612 commented Jul 26, 2017

grzesiek2010 commented Jul 26, 2017

shobhitagarwal1612 commented Jul 27, 2017

grzesiek2010 commented Jul 28, 2017

shobhitagarwal1612 commented Jul 31, 2017

grzesiek2010 commented Jul 31, 2017

lognaturel commented Aug 9, 2017

shobhitagarwal1612 commented Aug 9, 2017 •

edited

Loading

Sort menu redesign - Bottom Sheet Dialog #1242

Sort menu redesign - Bottom Sheet Dialog #1242

Conversation

shobhitagarwal1612 commented Jul 10, 2017

lognaturel left a comment

Choose a reason for hiding this comment

lognaturel Jul 11, 2017

Choose a reason for hiding this comment

shobhitagarwal1612 commented Jul 12, 2017

lognaturel commented Jul 14, 2017

mmarciniak90 commented Jul 19, 2017

shobhitagarwal1612 commented Jul 20, 2017

lognaturel commented Jul 25, 2017

grzesiek2010 commented Jul 25, 2017

grzesiek2010 commented Jul 25, 2017

shobhitagarwal1612 commented Jul 25, 2017

grzesiek2010 commented Jul 25, 2017

shobhitagarwal1612 commented Jul 26, 2017

grzesiek2010 commented Jul 26, 2017

shobhitagarwal1612 commented Jul 26, 2017

grzesiek2010 commented Jul 26, 2017

shobhitagarwal1612 commented Jul 27, 2017

grzesiek2010 commented Jul 28, 2017

shobhitagarwal1612 commented Jul 31, 2017

grzesiek2010 commented Jul 31, 2017

lognaturel commented Aug 9, 2017

shobhitagarwal1612 commented Aug 9, 2017 • edited Loading

shobhitagarwal1612 commented Aug 9, 2017 •

edited

Loading