Add toast to inform the user that download started. #5519

WoodyMats · 2021-01-31T22:52:53Z

What is it?

Bugfix (user facing)
Feature (user facing)
Codebase improvement (dev facing)
Meta improvement to the project (dev facing)

Description of the changes in your PR

Add toast to inform the user that download started
Add the right string in values folder
fixes Toast notification when download started #5428

Due diligence

I read the contribution guidelines.

WoodyMats · 2021-01-31T23:02:18Z

#5428 = Linked issue.

triallax · 2021-02-01T06:27:05Z

@WoodyMats thanks for opening a PR! Could you please not remove the template next time you open a PR?

triallax · 2021-02-01T06:27:52Z

app/src/main/res/values/strings.xml

    <string name="recent">Recent</string>
    <string name="chapters">Chapters</string>
    <string name="no_app_to_open_intent">No app on your device can open this</string>
+    <string name="download_has_started">Download has started</string>


Maybe it should be Downloading $VIDEO_NAME instead.

I don't see it as needed. The issue does not ask for this.

@WoodyMats thanks for opening a PR! Could you please not remove the template next time you open a PR?

Sorry! Now i just see the PR template. I modified my PR to contain the template. :)

XiangRongLin · 2021-02-01T06:48:32Z

Please don't open the PR against the release branch. It needs to be dev branch

triallax

Since your PR was initially opened against the release branch and then changed to the development branch, you still have a commit from the release branch (the commit that changes app/build.gradle). Could you revert that commit?

WoodyMats · 2021-02-02T13:59:40Z

Since your PR was initially opened against the release branch and then changed to the development branch, you still have a commit from the release branch (the commit that changes app/build.gradle). Could you revert that commit?

May i also add the Video name as you said in previous comment?

triallax · 2021-02-02T14:02:20Z

There isn't consensus on that, so you probably shouldn't add it for now.

WoodyMats · 2021-02-02T14:19:44Z

There isn't consensus on that, so you probably shouldn't add it for now.

Okk! The requested change just done. 😃

XiangRongLin

Other than that get the attention of the guy who opened the issue to test out the changes please.

Edit: and others who have shown interest in the issue being resolved

XiangRongLin · 2021-02-02T17:46:14Z

app/src/main/java/org/schabi/newpipe/download/DownloadDialog.java

                .apply();
+
+        Toast.makeText(context, getString(R.string.download_has_started),
+                Toast.LENGTH_LONG).show();


This is something someone with a better eye for UX can answer, but i don't think LENGTH_LONG (3.5 seconds) is necessary. LENGTH_SHORT (2 seconds) should be enough in my view.

https://stackoverflow.com/questions/7965135/what-is-the-duration-of-a-toast-length-long-and-length-short

You are right. I tried both and i think that LENGTH_SHORT looks more consistent. I'm changing it now.

WoodyMats · 2021-02-19T10:05:58Z

Good morning! 😄 I have changed the toast notification's duration from LENGTH_LONG to LENGTH_SHORT. You can check it! @mhmdanas

XiangRongLin · 2021-02-19T10:24:47Z

Other than that get the attention of the guy who opened the issue to test out the changes please.

Edit: and others who have shown interest in the issue being resolved

@WoodyMats see my last comment

Stypox · 2021-03-18T20:33:28Z

@XiangRongLin is there anything preventing this commit from being merged?

XiangRongLin · 2021-03-19T11:17:52Z

@Stypox Codewise nothing. As i said in my last comment it would be good to let the issue opener test out the changes. To see if it solves the problem for them.

Stypox · 2021-03-19T12:03:38Z

@illustribe could you test, please?

illustribe · 2021-03-19T12:58:04Z

@Stypox I've tested in version 0.20.11 and there's no toast message when one starts a download 🤷‍♂️ both while downloading audio or video...

triallax · 2021-03-19T13:11:38Z

@illustribe you're supposed to test this PR's APK, as this change has not been incorporated into 0.20.11. Here's the link.

illustribe · 2021-03-20T18:17:54Z

@Stypox it's working well! 👍

Add toast to inform the user that download started and add the right string in values.

Stypox · 2021-03-20T21:20:29Z

@WoodyMats @illustribe thank you!

…20.9 Add toast to inform the user that download started.

WoodyMats mentioned this pull request Jan 31, 2021

Toast notification when download started #5428

Closed

1 task

triallax reviewed Feb 1, 2021

View reviewed changes

Stypox changed the base branch from release_0.20.9 to dev February 1, 2021 07:33

WoodyMats requested a review from XiangRongLin February 2, 2021 09:38

triallax suggested changes Feb 2, 2021

View reviewed changes

XiangRongLin reviewed Feb 2, 2021

View reviewed changes

triallax added the GUI Issue is related to the graphical user interface label Feb 3, 2021

TobiGr force-pushed the dev branch 2 times, most recently from 679bc75 to 2aeccc0 Compare March 16, 2021 08:24

Stypox force-pushed the release_0.20.9 branch from eb698ba to d22604c Compare March 20, 2021 21:16

Show Toast when download starts

bf6645e

Add toast to inform the user that download started and add the right string in values.

Stypox force-pushed the release_0.20.9 branch from d22604c to bf6645e Compare March 20, 2021 21:18

Stypox merged commit c7efa8c into TeamNewPipe:dev Mar 20, 2021

This was referenced Mar 21, 2021

Release 0.21.0 #5871

Merged

Release 0.21.0 #5872

Closed

ghost mentioned this pull request Apr 11, 2021

request on when a future update of the legacy version of newpiepe will arrive TeamNewPipe/NewPipe-legacy#72

Open

tossj pushed a commit to tossj/NewPipe-legacy that referenced this pull request Apr 22, 2021

Merge pull request TeamNewPipe/NewPipe#5519 from WoodyMats/release_0.…

7068fc3

…20.9 Add toast to inform the user that download started.

subraizada3 mentioned this pull request Apr 22, 2021

'Download has started' toast notification sometimes appears when it shouldn't #6138

Closed

4 tasks

Uh oh!

Add toast to inform the user that download started. #5519

Add toast to inform the user that download started. #5519

Uh oh!

Conversation

WoodyMats commented Jan 31, 2021 • edited by Stypox Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is it?

Description of the changes in your PR

Due diligence

Uh oh!

WoodyMats commented Jan 31, 2021

Uh oh!

triallax commented Feb 1, 2021

Uh oh!

triallax Feb 1, 2021

Choose a reason for hiding this comment

Uh oh!

XiangRongLin Feb 1, 2021

Choose a reason for hiding this comment

Uh oh!

WoodyMats Feb 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

XiangRongLin commented Feb 1, 2021

Uh oh!

triallax left a comment

Choose a reason for hiding this comment

Uh oh!

WoodyMats commented Feb 2, 2021

Uh oh!

triallax commented Feb 2, 2021

Uh oh!

WoodyMats commented Feb 2, 2021

Uh oh!

XiangRongLin left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

XiangRongLin Feb 2, 2021

Choose a reason for hiding this comment

Uh oh!

WoodyMats Feb 4, 2021

Choose a reason for hiding this comment

Uh oh!

WoodyMats commented Feb 19, 2021

Uh oh!

XiangRongLin commented Feb 19, 2021

Uh oh!

Stypox commented Mar 18, 2021

Uh oh!

XiangRongLin commented Mar 19, 2021

Uh oh!

Stypox commented Mar 19, 2021

Uh oh!

illustribe commented Mar 19, 2021

Uh oh!

triallax commented Mar 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

illustribe commented Mar 20, 2021

Uh oh!

Stypox commented Mar 20, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

WoodyMats commented Jan 31, 2021 •

edited by Stypox

Loading

WoodyMats Feb 1, 2021 •

edited

Loading

XiangRongLin left a comment •

edited

Loading

triallax commented Mar 19, 2021 •

edited

Loading