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

Allow user to pin their favorite sites in Brave Payments (and other UI tweaks) #7347

Closed
bradleyrichter opened this issue Feb 21, 2017 · 12 comments

Comments

@bradleyrichter
Copy link
Contributor

bradleyrichter commented Feb 21, 2017

Automated test plan

  • npm run test -- --grep="LedgerTable"
  • npm run test -- --grep="Ledger table"

Test plan

Original issue description

image

Item 1: Update column headers to match this config. The rank number column has been removed, and an actions column has been added.

Item 2: Create a new grouped column for action buttons. The delete button moves here, and "Pin site" will be added. Eventually, a tweet button may be added.

Item 3: The percent column needs to support an edit box with active and inactive states.

Item 4: Add ability to show permanent positioned items by adding sort hierarchy. This will allow sorting by "pinned state" and then column.

@NejcZdovc
Copy link
Contributor

@bradleyrichter
User will be able to set percentage only for pinned publishers?

How will the user set default value again? Just delete everything in the input and press enter/unselect and default value will be inserted?

@NejcZdovc
Copy link
Contributor

When user deletes one publisher, how can he retrieve it back?
I noticed that include is disabled when item is pinned, so user can't toggle it when publisher is pinned?
@bradleyrichter

NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Mar 9, 2017
@NejcZdovc NejcZdovc mentioned this issue Mar 9, 2017
4 tasks
@mrose17
Copy link
Member

mrose17 commented Mar 9, 2017

if you remove a publisher (e.g., click under the "Remove" column -or- right-click for "Never include this site...") you can re-enable the publisher by going to shields and removing the blocking entry there.

note that this won't recover the previous history ... that's gone forever, by design. it will allow the publisher to be considered going forward.

This was referenced Mar 10, 2017
@NejcZdovc
Copy link
Contributor

@mrose17 @bradleyrichter I am testing the current version I noticed that when you exclude one site, this site still have percentage. Shouldn't excluded site percentage be set to 0 and percentages for other sites be recalculated?

@bradleyrichter
Copy link
Contributor Author

bradleyrichter commented Mar 10, 2017 via email

@NejcZdovc
Copy link
Contributor

NejcZdovc commented Mar 10, 2017

@bradleyrichter Ok, so I need to change this as well I guess. Will do

@NejcZdovc NejcZdovc modified the milestones: 0.13.7, 0.13.6 Mar 12, 2017
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Mar 14, 2017
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Mar 15, 2017
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Mar 19, 2017
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Mar 20, 2017
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Mar 22, 2017
Resolves brave#7347

Auditors: @bsclifton @bradleyrichter @mrose17

Test plan:
- npm run test -- --grep="LedgerTable"
- npm run test -- --grep="Ledger table"
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Mar 22, 2017
Resolves brave#7347
Resolves brave#6890

Auditors: @bsclifton @bradleyrichter @mrose17

Test plan:
- npm run test -- --grep="LedgerTable"
- npm run test -- --grep="Ledger table"
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Mar 22, 2017
Resolves brave#7347
Resolves brave#6890

Auditors: @bsclifton @bradleyrichter @mrose17

Test plan:
- npm run test -- --grep="LedgerTable"
- npm run test -- --grep="Ledger table"
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Mar 22, 2017
Resolves brave#7347
Resolves brave#6890

Auditors: @bsclifton @bradleyrichter @mrose17

Test plan:
- npm run test -- --grep="LedgerTable"
- npm run test -- --grep="Ledger table"
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Mar 23, 2017
Resolves brave#7347
Resolves brave#6890

Auditors: @bsclifton @bradleyrichter @mrose17

Test plan:
- npm run test -- --grep="LedgerTable"
- npm run test -- --grep="Ledger table"
@bsclifton
Copy link
Member

@NejcZdovc could you post the steps to test in the original post here? Thanks 😄

bsclifton added a commit that referenced this issue Mar 30, 2017
bridiver pushed a commit that referenced this issue Apr 4, 2017
Resolves #7347
Resolves #6890

Auditors: @bsclifton @bradleyrichter @mrose17

Test plan:
- npm run test -- --grep="LedgerTable"
- npm run test -- --grep="Ledger table"
@bsclifton bsclifton changed the title Update ledger list control to support new features for 1.0 Allow user to pin their favorite sites in Brave Payments (and other UI tweaks) Apr 14, 2017
@alexwykoff alexwykoff changed the title Allow user to pin their favorite sites in Brave Payments (and other UI tweaks) Added user pinning in Brave Payments Apr 25, 2017
@luixxiul luixxiul changed the title Added user pinning in Brave Payments Allow user to pin their favorite sites in Brave Payments (and other UI tweaks) May 29, 2017
@luixxiul
Copy link
Contributor

@NejcZdovc with the commit you closed the issue you deleted minimumPercentage and I am not sure why.

3515e99#diff-6e336f58ebfb0aaf5268c68f492d5c9cL64

@luixxiul luixxiul added the needs-info Another team member needs information from the PR/issue opener. label May 29, 2017
@NejcZdovc
Copy link
Contributor

@luixxiul because we added show all button instead

@luixxiul
Copy link
Contributor

luixxiul commented May 29, 2017

and we cannot hide 0 % sites once you push the button as we don't have the hide button

which is by design?

@luixxiul
Copy link
Contributor

luixxiul commented May 29, 2017

As a result of that:

alot

If it is by design, let's add bulk remove function

@NejcZdovc
Copy link
Contributor

Let's continue the discussion in #9137

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants