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

Confirmation dialog when accepting / rejecting / delete request #826

Closed
wants to merge 5 commits into from
Closed

Conversation

Hyper-Matrix
Copy link
Contributor

@Hyper-Matrix Hyper-Matrix commented Sep 1, 2020

Description

I have added the Alert dialogues for accepting, rejecting, and deleting requests.

Fixes #250

Type of Change:

Delete irrelevant options.

  • Code

Code/Quality Assurance Only

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

How Has This Been Tested?

Tested on my device (Redmi Note 5 pro).

Screenshot -
Screenshot_2020-09-01-13-48-12-630_org systers mentorship
Screenshot_2020-09-04-20-38-17-018_org systers mentorship
Screenshot_2020-09-04-20-38-21-183_org systers mentorship

Checklist:

Delete irrelevant options.

  • 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
  • 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
  • Any dependent changes have been published in downstream modules

@Hyper-Matrix Hyper-Matrix changed the title Local confirmation dialog when accepting / rejecting / delete request Sep 1, 2020
@Hyper-Matrix Hyper-Matrix changed the title confirmation dialog when accepting / rejecting / delete request Confirmation dialog when accepting / rejecting / delete request Sep 1, 2020
@rpattath
Copy link
Member

rpattath commented Sep 2, 2020

@Hyper-Matrix please squash your commits as mentioned in contributing guidelines

@Hyper-Matrix
Copy link
Contributor Author

@rpattath I am really confused about how to squash a PR. Can you please help me out with the same?

@rpattath
Copy link
Member

rpattath commented Sep 2, 2020

@Hyper-Matrix
Copy link
Contributor Author

Hyper-Matrix commented Sep 3, 2020

Yes @rpattath it does help. But the issue is neither my android studio terminal nor my cmd(for git bash) works. Whenever I try to open them it crashes. So I use github desktop. So is there anyway I can implement this using github desktop ?

@anna4j anna4j added the Status: Needs Testing Work has been reviewed and needs the code tested by the quality assurance team. label Sep 3, 2020
@rpattath rpattath added Status: Needs Review PR needs an additional review or a maintainer's review. and removed Status: Needs Testing Work has been reviewed and needs the code tested by the quality assurance team. labels Sep 4, 2020
@rpattath
Copy link
Member

rpattath commented Sep 4, 2020

Awesome work @Hyper-Matrix ! Could you attach screenshots of the dialogue shown when accepting and rejecting the request.

@Hyper-Matrix
Copy link
Contributor Author

@rpattath Thanks.......I have attached it.

Copy link
Member

@rpattath rpattath left a comment

Choose a reason for hiding this comment

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

LGTM

@rpattath rpattath added Status: Needs Testing Work has been reviewed and needs the code tested by the quality assurance team. and removed Status: Needs Review PR needs an additional review or a maintainer's review. labels Sep 4, 2020
@rpattath
Copy link
Member

@Hyper-Matrix which branch have you created this PR from ? It is showing unknown repo.

@Hyper-Matrix
Copy link
Contributor Author

@rpattath Even I can't understand whats the issue. I have the repo in my github. What should I do now? A new PR?

@Hyper-Matrix
Copy link
Contributor Author

@rpattath Can't it be merged if it remains unknown?

@vj-codes
Copy link
Member

@Hyper-Matrix hello , after creating this PR did you delete your fork and then re-forked it again?
Unkown repo is generally caused by this action

@Hyper-Matrix
Copy link
Contributor Author

@vj-codes Yeah, I actually realized that...So I asked...I think it can be merged so do I need to put up another PR?

@vj-codes
Copy link
Member

@Hyper-Matrix before merging all PRs are tested by QA team , but since this PR does not have any specific repo and branch , testing will not be possible/not successful.
It is highly advisible to create a new PR with same changes to avoid problems later.
Also remember while creating a new branch make sure you are on develop branch before git checkout .Otherwise commits from other PRs will show up in the new one.
What say ? @Hyper-Matrix
cc : @rpattath

@Hyper-Matrix
Copy link
Contributor Author

@vj-codes Is there any other way out? Else I will submit a new PR.

@vj-codes
Copy link
Member

@anna4j what do you suggest ? ^^^

@vj-codes vj-codes removed the Status: Needs Testing Work has been reviewed and needs the code tested by the quality assurance team. label Sep 21, 2020
@vj-codes vj-codes added Help Wanted Extra attention is needed. Status: Needs Review PR needs an additional review or a maintainer's review. labels Sep 21, 2020
@rpattath
Copy link
Member

@Hyper-Matrix please submit a new PR. Make sure you use create it from a local branch under your fork and not from develop branch.

@rpattath rpattath added Status: Changes Requested Changes are required to be done by the PR author. and removed Help Wanted Extra attention is needed. Status: Needs Review PR needs an additional review or a maintainer's review. labels Sep 21, 2020
@Hyper-Matrix
Copy link
Contributor Author

@rpattath just to make sure I had already submitted the doc for OSH. After this PR will it be fine to submit it again?

@Hyper-Matrix
Copy link
Contributor Author

@rpattath @anna4j I have added a new PR #949 please review it.

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.

Create a confirmation dialog when accepting / rejecting / delete request
4 participants