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

feat: New Window for Searching Members #1174

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

bosankus
Copy link

Description

Created a separate fragment for searching members i.e SearchMembersFragment.
This solves the below mentioned issue, and is implemented as per the discussion on issue #719.

Fixes #732

Type of Change:

  • Code
  • User Interface

Code/Quality Assurance Only

  • New feature (non-breaking change which adds functionality pre-approved by mentors)

How Has This Been Tested?

Checklist:

  • My PR follows the style guidelines of this project
  • I have performed a self-review of my own code or materials
  • I have commented my code or provided relevant documentation, particularly in hard-to-understand areas

Code/Quality Assurance Only

  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works - Espresso testing added but needs to be reviewed
  • New and existing unit tests pass locally with my changes

- SearchMembersFragment.kt will get user list to search from SingletonUserList.kt which is set from MembersFragment.kt
- few strings replaced with strings.xml
- search member method removed from MembersFragment.kt
- menu search icon onclick will replace with SearchMembersFragment.kt

NB: Spotless check success. Commit does not include tests
- rearranged methods for readability in SearchMembersFragment.kt
- spotless errors fixed
- corrected strings.xml search view hint
@bosankus
Copy link
Author

@isabelcosta @epicadk facing one issue in this PR. That is the added espresso testing. Though the feature works fine. It will be helpful if any of you can help me to review the testing part.
Also I will be creating a new issue for the back press issue which is getting faced in this new screen. 'Cancel' button removes the fragment and works smooth.
Kindly share any ideas regarding this.

@bosankus bosankus changed the title New Window for Searching Members feat: New Window for Searching Members Nov 12, 2021
@epicadk
Copy link
Member

epicadk commented Nov 12, 2021

@isabelcosta @epicadk facing one issue in this PR. That is the added espresso testing. Though the feature works fine. It will be helpful if any of you can help me to review the testing part. Also I will be creating a new issue for the back press issue which is getting faced in this new screen. 'Cancel' button removes the fragment and works smooth. Kindly share any ideas regarding this.

Yeah for some reason I couldn't get the tests to work either. @isabelcosta do you think we could create an issue for this and perhaps only manually test it now?

@bosankus
Copy link
Author

@isabelcosta can you check kindly?

@vj-codes
Copy link
Member

@bosankus thank you for your inputs, do create the issue if you have time as you have got a better understanding of it, if not that's alright as well.
We can proceed with manual testing of the feature!

@vj-codes vj-codes added Category: Coding Changes to code base or refactored code that doesn't fix a bug. Status: Needs Review PR needs an additional review or a maintainer's review. labels Nov 18, 2021
@bosankus
Copy link
Author

@vj-codes Sure, I will create the issue.

@bosankus
Copy link
Author

bosankus commented Dec 3, 2021

This feature will be improved due to implementation of paging3 in MembersFragment from issue #1166.
Closing this issue due to duplicate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Coding Changes to code base or refactored code that doesn't fix a bug. Status: Needs Review PR needs an additional review or a maintainer's review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New Window for Searching Members
3 participants