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

Consistent usage of dialogs in the app #871

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

Conversation

cdhiraj40
Copy link
Contributor

@cdhiraj40 cdhiraj40 commented Dec 7, 2021

fixes #860 & addition to PR #747

69014902-3c5e-4bfe-87bf-70a09b8a89b5

81a35d67-e6af-4f45-bc85-288ac1fedcf9

Copy link
Member

@asaikarthikeya asaikarthikeya left a comment

Choose a reason for hiding this comment

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

Hey @cdhiraj40. Kindly refer #752. Please use this format for the dialogs with progress bars. Thanks :)

@cdhiraj40
Copy link
Contributor Author

cdhiraj40 commented Dec 7, 2021

Hey @cdhiraj40. Kindly refer #752. Please use this format for the dialogs with progress bars. Thanks :)

sure will do :)
but I have one doubt as the dialog box used earlier for uploading all tracks had percentage show below which used to update, so do I have to add here? (it used to update using custom view)
image

@asaikarthikeya
Copy link
Member

@cdhiraj40 I see that there are 2 dialogs, 1 for uploading all tracks which uses percentage. And another dialog which uploads 1 track and it has circular progress bar. Am i right? You can prefer the procedure used in #752 for single track uploading dialog and for all tracks keep the percentage dialog.

@cdhiraj40
Copy link
Contributor Author

@cdhiraj40 I see that there are 2 dialogs, 1 for uploading all tracks which uses percentage. And another dialog which uploads 1 track and it has circular progress bar. Am i right? You can prefer the procedure used in #752 for single track uploading dialog and for all tracks keep the percentage dialog.

sure, will update the PR soon :)

@asaikarthikeya
Copy link
Member

Hey @cdhiraj40, Just a small suggestion i would like to add. Kindly change the PR title here, as this coincides with another PR- #747. Mentioning the older PR number in this PR is sufficent to state that both are related.

Also, when you add new changes after reviewing, do specify the change as the commit message. please refrain using "requested changes" as the commit message. Commit messages should specify the changes in that commit. requested changes doesnot specify the changes clearly. Thanks :)

@cdhiraj40
Copy link
Contributor Author

Hey @cdhiraj40, Just a small suggestion i would like to add. Kindly change the PR title here, as this coincides with another PR- #747. Mentioning the older PR number in this PR is sufficent to state that both are related.

sure, I will do it so that there is no confusion :)

Also, when you add new changes after reviewing, do specify the change as the commit message. please refrain from using "requested changes" as the commit message. Commit messages should specify the changes in that commit. requested changes do not specify the changes clearly.

Sure my bad, that is indeed a good suggestion I will try to take care from here. Thanks for this.
I will revert and commit the change soon :)

@cdhiraj40 cdhiraj40 changed the title Consistent dialog's and removing the use of deprecated MaterialDialog.Builder Consistent usage of dialogs in the app Dec 13, 2021
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.

2 participants