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

fix:Add toast message when no user with common skills is found using filter(anita-org#866) #884

Closed
wants to merge 47 commits into from

Conversation

ritvij14
Copy link

Description

Added Toast for the situation when members with common skills are not found.

Fixes #866

Type of Change:

  • Code

Code/Quality Assurance Only

  • Bug-fix (non-breaking change which fixes an issue)

How Has This Been Tested?

I tested it manually on my device Redmi Note 8 Pro. Video

Checklist:

Delete irrelevant options.

  • My PR follows the style guidelines of this project
  • I have performed a self-review of my own code or materials
  • Any dependent changes have been merged

Code/Quality Assurance Only

  • My changes generate no new warnings
  • New and existing unit tests pass locally with my changes

sanchi0204
sanchi0204 previously approved these changes Sep 10, 2020
anna4j
anna4j previously approved these changes Sep 13, 2020
@anna4j anna4j added Status: Needs Testing Work has been reviewed and needs the code tested by the quality assurance team. and removed Open Source Hack labels Sep 13, 2020
@spacemonkey1101
Copy link
Contributor

is this issue open?

Copy link
Member

@mtreacy002 mtreacy002 left a comment

Choose a reason for hiding this comment

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

@ritvij14 , I've tested this PR locally on a Pixel 2 emulator. I found a bug in the code that the toast Ooops! Not found pop up not only when the filter is applied, but even before that (when logged-in user select Members icon from the bottom Navbar). Can you please fix this? Thanks

ezgif com-gif-maker (8)

cc @anna4j and @sanchi0204

@mtreacy002 mtreacy002 added Status: Changes Requested Changes are required to be done by the PR author. and removed Status: Needs Testing Work has been reviewed and needs the code tested by the quality assurance team. labels Oct 15, 2020
@ritvij14
Copy link
Author

ritvij14 commented Oct 17, 2020

@mtreacy002 sorry for the late response 😅

Could you please guide me as to where should I add the toast to not face this bug? I am really confused.

@mtreacy002
Copy link
Member

@ritvij14 , when you run the app with the debugger, you can see that on its initial state when user first click the Members page (providing the app is newly installed, to avoid getting side effect from cache of previous actions), the filterMap on line 119 of MembersFragment will show only '"sort_key" -> "REGISTRATION_DATE"` as the filter condition.
Screen Shot 2020-10-18 at 4 38 46 pm

Then when you type in a specific filter, the filterMap will show 3 other attributes with "skills" showing the string you just typed (in my example its "python").
Screen Shot 2020-10-18 at 4 40 08 pm

Even if user clear all filters, the filterMap will show all 4 attributes (sort_key + 3 filters) but with filters getting empty strings.
Screen Shot 2020-10-18 at 4 46 43 pm

You can use this information to place an if condition to check the status of filterMap and apply toast only when the desired filter exist in filterMap and not an empty string.

Hope this helps

@ritvij14
Copy link
Author

@mtreacy002 thanks for the clarification, I will add an if condition for the filters. But this bug didn't happen on my device, is that because of different data in the recycler view or something else?

20201018_124650

@ritvij14 ritvij14 dismissed stale reviews from anna4j and sanchi0204 via e8089dd October 18, 2020 07:29
Copy link
Member

@mtreacy002 mtreacy002 left a comment

Choose a reason for hiding this comment

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

@ritvij14 . I tested the latest change but the toast ended up not showing at all, both on initial click to Members page and after the filter is applied.

  • tested with debug_localhost as active build variant
    pr-884

  • tested with debug as active build variant
    pr-884b

Can you please fix this?

@ritvij14
Copy link
Author

@mtreacy002 yeah sorry about that 😅 I didn't notice that I had placed the wrong condition there. I have changed it and this is the current look. Does it look ok now?

20201024_123456 1

@vj-codes vj-codes removed Open Source Hack Status: Changes Requested Changes are required to be done by the PR author. labels Mar 2, 2021
CodeChamp-SS
CodeChamp-SS previously approved these changes Mar 4, 2021
Copy link
Contributor

@CodeChamp-SS CodeChamp-SS left a comment

Choose a reason for hiding this comment

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

LGTM ! Tested locally on emulator as well. Thanks @ritvij14 for your contribution.

@vj-codes
Copy link
Member

vj-codes commented Mar 4, 2021

@CodeChamp-SS can you please add a gif if you've tested on emulator just to crosscheck it's functionality, thanks!

@CodeChamp-SS
Copy link
Contributor

CodeChamp-SS commented Mar 5, 2021

@CodeChamp-SS can you please add a gif if you've tested on emulator just to crosscheck it's functionality, thanks!

on testing in different scenarios, I found a bug 😅. Even when the result is found, the toast pops up.

@ritvij14 can you please fix it ? I'll be happy to test again once you've fixed this issue :-)

pr884.testing.mp4

@ritvij14
Copy link
Author

ritvij14 commented Mar 5, 2021

@CodeChamp-SS yes ok I will have a look once again and get back to you, will try to correct the errors asap.

@vj-codes
Copy link
Member

vj-codes commented Mar 5, 2021

@CodeChamp-SS thanks for uploading it :)

@vj-codes vj-codes added Status: Changes Requested Changes are required to be done by the PR author. and removed Status: Needs Review PR needs an additional review or a maintainer's review. labels Mar 5, 2021
@ritvij14
Copy link
Author

ritvij14 commented Mar 8, 2021

@CodeChamp-SS in the most recent pull of the latest code, my Gradle build is getting stopped and failing showing this error:-
Gradle sync failed: Gradle build daemon disappeared unexpectedly

This is not happening in other android projects on my system.

@CodeChamp-SS
Copy link
Contributor

@CodeChamp-SS in the most recent pull of the latest code, my Gradle build is getting stopped and failing showing this error:-
Gradle sync failed: Gradle build daemon disappeared unexpectedly

This is not happening in other android projects on my system.

do one thing revert all the commits from your branch, then pull the develop branch, then commit your changes. Then try to build, ig that should fix the problem. If your changes are working as expected then do force push on the branch

@ritvij14
Copy link
Author

ritvij14 commented Mar 8, 2021

@CodeChamp-SS in the most recent pull of the latest code, my Gradle build is getting stopped and failing showing this error:-
Gradle sync failed: Gradle build daemon disappeared unexpectedly
This is not happening in other android projects on my system.

do one thing revert all the commits from your branch, then pull the develop branch, then commit your changes. Then try to build, ig that should fix the problem. If your changes are working as expected then do force push on the branch

Would that fix the issue though? Because I have this on my local copy's develop branch too and it is completely even with the current develop branch.

@CodeChamp-SS
Copy link
Contributor

@CodeChamp-SS in the most recent pull of the latest code, my Gradle build is getting stopped and failing showing this error:-
Gradle sync failed: Gradle build daemon disappeared unexpectedly
This is not happening in other android projects on my system.

do one thing revert all the commits from your branch, then pull the develop branch, then commit your changes. Then try to build, ig that should fix the problem. If your changes are working as expected then do force push on the branch

Would that fix the issue though? Because I have this on my local copy's develop branch too and it is completely even with the current develop branch.

hmm then ig you can try cleaning and then rebuilding the project, if the problem still exists i think you'll have to delete the project and then clone it again in your machine, this the only solution i could think of 😅 as i've never faced such an issue

@CodeChamp-SS
Copy link
Contributor

you may also try asking on zulip may be some1else could be able to help you :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Changes Requested Changes are required to be done by the PR author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add toast message when no user with common skills is found using filter