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

Closes #672 Downloads support #3072

Merged
merged 5 commits into from Apr 13, 2020
Merged

Closes #672 Downloads support #3072

merged 5 commits into from Apr 13, 2020

Conversation

keianhzo
Copy link
Collaborator

@keianhzo keianhzo commented Mar 30, 2020

Closes #672 This PR adds Downloads support. Some refactoring has been done to take all the Library panels logic out from the Window class.

Waiting for final design.

@keianhzo keianhzo self-assigned this Mar 30, 2020
@keianhzo keianhzo added this to the #10 features milestone Mar 30, 2020
@keianhzo keianhzo changed the title Downloads Closes #672 Downloads support Apr 1, 2020
@bluemarvin
Copy link
Contributor

Seems pretty good. Not sure I like the badge. I also find the save location of internal or external confusing. I have no idea where that would be if I went looking for the files on the device if I were to mount it.

@keianhzo
Copy link
Collaborator Author

keianhzo commented Apr 7, 2020

Updated with final design: https://trello.com/c/iBLotKN4

@keianhzo keianhzo marked this pull request as ready for review April 7, 2020 10:54
@bluemarvin
Copy link
Contributor

Looks like the download icon is still missing?
org mozilla vrbrowser-20200407-170240

@bluemarvin
Copy link
Contributor

The spec also has the download progress like on desktop with a blue badge after download is complete.

@keianhzo
Copy link
Collaborator Author

keianhzo commented Apr 8, 2020

Not sure why you don't see them but both things you mentioned were already there in the last push. I've forced push again and fixed and issue I was seeing with clipping.

Copy link
Contributor

@bluemarvin bluemarvin left a comment

Choose a reason for hiding this comment

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

Download window fails to show if page creates a new tab. STR:

  1. go to https://community-tc.services.mozilla.com/tasks/Vl6vfgK4Ti6y9M1aMl65rQ#artifacts
  2. click on one of the APKs and start download
  3. click download button from tray

Actual:
Nothing happens

Expected:
Download window is opened.

@keianhzo keianhzo force-pushed the v10/downloads branch 2 times, most recently from 94b584e to 939fcc3 Compare April 9, 2020 09:45
@keianhzo keianhzo requested a review from bluemarvin April 9, 2020 09:45
@keianhzo
Copy link
Collaborator Author

keianhzo commented Apr 9, 2020

@bluemarvin seems to be related to the firstpaint issue.

@bluemarvin
Copy link
Contributor

@bluemarvin seems to be related to the firstpaint issue.

Which first paint issue? Has it been fixed in master because I rebased and am still seeing the issue.

@keianhzo
Copy link
Collaborator Author

keianhzo commented Apr 9, 2020

@bluemarvin I didn't have time to fix the firstpaint issue today but what you see here it's related to that. You can also reproduce it if you open a new tab in the background from the context menu and switch to that tab.

@bluemarvin bluemarvin merged commit ee7d323 into master Apr 13, 2020
@bluemarvin bluemarvin deleted the v10/downloads branch April 13, 2020 16:06
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.

Implement downloads to device
2 participants