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

Adds Notification List on HomePage with sorting and filtering options. #31

Merged
merged 3 commits into from
Jul 3, 2019

Conversation

DreamyPhobic
Copy link
Collaborator

Closes #30

@jbeorse
Copy link
Collaborator

jbeorse commented Jun 15, 2019

Does this show all messages from all groups, or does it pre-filter based on the groups that the ODK user belongs to? For example, imagine there are groups: north_clinic, east_clinic. Imagine we have users north_admin (part of north_clinic group), east_admin(part of east_clinic group), and north_east_director (part of north_clinic and east_clinic groups).

If I'm logged in as north_clinic I would expect to only see messages for the north_clinic group (and all other groups messages would be inaccessible to me). If I'm logged in as east_clinic I would expect to only see messages for east_clinic group. If I'm logged in as north_east_director I would expect to see messages from both groups.

Does this PR add the option for noth_east_director to filter based on the multiple groups that he is a part of?

@DreamyPhobic
Copy link
Collaborator Author

Yes initially it will show all the messages of groups which are subscribed by that user. And by clicking on filter, user can filter messages by group.

@@ -18,4 +18,11 @@
<string name="error">Error</string>
<string name="json_file_missing_error_message">Server has not been configured correctly. Please, check the server settings in ODK Services.</string>

<!-- TODO: Remove or change this placeholder text -->
<string name="hello_blank_fragment">Hello blank fragment</string>
Copy link
Member

Choose a reason for hiding this comment

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

This value is not used.

Copy link
Member

@linl33 linl33 left a comment

Choose a reason for hiding this comment

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

The fragments for sort and filter cause a crash when Android needs to tear down and recreate them, for example on screen rotation. The problem is that they have a non zero-argument constructor, fragments cannot have these constructors. See Fragment documentation for more information. To supply parameters to the fragments, they should be supplied with the setArgument method. See the DetailsFragment here for an example provided by Google. It uses a static method in the fragment to ensure all parameters are set.

The strings added should be moved to strings.xml

2 more things that are nice to have but they shouldn't block this merge.

  • The search function launches a new instance of MainActivity, so each search creates an entry on the backstack. This is counter intuitive and generally not how this kind of search is implemented. @jbeorse what do you think the desired behavior should be?
  • It would be nice if the sort menu can remember the selected option.

@DreamyPhobic
Copy link
Collaborator Author

@linl33 @jbeorse I have implemented the search functionality according to the developer guide https://developer.android.com/guide/topics/search/search-dialog.
And I think it's not creating any extra instance of MainActivity, I have also checked with the android profiler.

@linl33 linl33 merged commit 502de78 into odk-x:master Jul 3, 2019
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.

Home Screen is empty.
3 participants