Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Converts DownloadItem and DownloadsBar into redux #9271

Merged
merged 1 commit into from
Jun 7, 2017

Conversation

NejcZdovc
Copy link
Contributor

@NejcZdovc NejcZdovc commented Jun 5, 2017

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.

Resolves #9260

Auditors: @bsclifton

Test Plan:

Reviewer Checklist:

Tests

  • Adequate test coverage exists to prevent regressions
  • Tests should be independent and work correctly when run individually or as a suite ref
  • New files have MPL2 license header

@NejcZdovc NejcZdovc added this to the 0.18.x milestone Jun 5, 2017
@NejcZdovc NejcZdovc self-assigned this Jun 5, 2017
@luixxiul
Copy link
Contributor

luixxiul commented Jun 5, 2017

would you creating automated tests or a manual test plan (as this is not just a copy-and-paste change)?

@NejcZdovc
Copy link
Contributor Author

will do after this PR is completed

@@ -92,11 +126,11 @@ describe('downloadsBar component', function () {
})
})

describe('very narrow downloads bar with items', function () {
// skipped until #9176 is merged
describe.skip('very narrow downloads bar with items', function () {
Copy link
Collaborator

Choose a reason for hiding this comment

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

skip?

Copy link
Contributor Author

@NejcZdovc NejcZdovc Jun 6, 2017

Choose a reason for hiding this comment

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

I skipped this last one, because it uses computes styles and I am working on another PR that will change that in the same version (0.18.x). So that I don't need to change it twice I will fix it in #9176 after this one is merged.

Copy link
Contributor

Choose a reason for hiding this comment

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

@NejcZdovc would you please add that to #5389 for a reminder?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@luixxiul no need, because I will include it in #9176, that will be part of 0.18 as well

Copy link
Contributor

Choose a reason for hiding this comment

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

it's fine as long as you won't forget.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I won't 😄

@bridiver
Copy link
Collaborator

bridiver commented Jun 6, 2017

just wondering why the last test is skipped, but otherwise ++

@NejcZdovc NejcZdovc force-pushed the redux/downloads branch 2 times, most recently from 5584d38 to 32bad94 Compare June 6, 2017 22:33
Resolves brave#9260

Auditors: @bsclifton

Test Plan:
- download an item
- check if download item is shown
- check if you can interact with it (note there is a problem with pause/resume, fixed here brave#9246)
@NejcZdovc NejcZdovc merged commit c4c18ee into brave:master Jun 7, 2017
@luixxiul luixxiul changed the title Coverts DownloadItem and DownloadsBar into redux Converts DownloadItem and DownloadsBar into redux Jun 7, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Redux component - Download area
3 participants