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

Add "hide sites if" options to Advanced Options Dialog #4681

Closed
bradleyrichter opened this issue Oct 11, 2016 · 18 comments
Closed

Add "hide sites if" options to Advanced Options Dialog #4681

bradleyrichter opened this issue Oct 11, 2016 · 18 comments
Assignees
Labels
Milestone

Comments

@bradleyrichter
Copy link
Contributor

As a user, I want the option to filter/minimize the amount of less important sites in my Brave Payments list.

First option - "Hide sites with less than 1% usage"

This will reduce the total number of sites, and only show sites that have a high probability of receiving a vote on my reconciliation day.

This option is ON by default.

Second option - "Hide sites that I have disabled"

This will keep my list tidy by hiding each site that I have chosen not to include by flipping the include switch.

With this option enabled, the site should disappear from the list view 1 second after I flip the switch to "don't include". If I wish to change or review the sites I have disabled, I can return to advanced settings and change this option to OFF.

This option is OFF by default.

image

@bradleyrichter bradleyrichter added design A design change, especially one which needs input from the design team. feature/rewards labels Oct 11, 2016
@bradleyrichter bradleyrichter added this to the 0.12.5dev milestone Oct 11, 2016
@bradleyrichter
Copy link
Contributor Author

CC @mrose17

@mrose17 mrose17 modified the milestones: 0.12.6dev, 0.12.5dev Oct 11, 2016
@diracdeltas diracdeltas self-assigned this Oct 11, 2016
@mrose17 mrose17 modified the milestones: 0.12.7dev, 0.12.6dev, 0.12.8dev Oct 17, 2016
@mrose17 mrose17 modified the milestones: 1.1.0, 0.12.8dev Oct 25, 2016
@luixxiul
Copy link
Contributor

Designed. How is this?

screenshot 2016-12-11 0 21 27

@bradleyrichter
Copy link
Contributor Author

@luixxiul Looks good except for the drop down style needs the new style.

And, the switches at the right are implemented too?

@mrose17
Copy link
Member

mrose17 commented Dec 10, 2016

a few nits:

  • s/hide/Hide/g
  • s/site that/sites that/

@bradleyrichter
Copy link
Contributor Author

Actually, "Hide sites that I have disabled" has been moved out to the ledger list area.

image

@bradleyrichter
Copy link
Contributor Author

see #6101

@luixxiul
Copy link
Contributor

OK, I updated.

screenshot 2016-12-11 12 34 22

I just copied the switch as I'm not sure how to implement prefKey for it.

@luixxiul
Copy link
Contributor

luixxiul commented Dec 11, 2016

@bradleyrichter

Looks good except for the drop down style needs the new style.

It will be applied after #6084 is merged (the margin-bottom of the labels will be applied with f8efaca of #6047).

With these (and the commits which are yet to be pushed) the modal dialog will be like this:
screenshot 2016-12-12 0 19 53

@luixxiul
Copy link
Contributor

Would someone help me to implement the switch?

I'm trying to hide sites with less than 1%. I added following the other switch in preferences.js:

<SettingCheckbox
 dataL10nId='minimumPercentage'
 prefKey={settings.MINIMUM_PERCENTAGE}
 settings={this.props.settings}
 onChangeSetting={this.props.onChangeSetting} />

In js/constants/appConfig:

advanced.minimum-percentage': false,

In js/constants/settings.js:

MINIMUM_PERCENTAGE: 'advanced.minimum-percentage',

@luixxiul luixxiul added the help wanted The PR/issue opener needs help to complete/report the task. label Dec 12, 2016
@mrose17
Copy link
Member

mrose17 commented Dec 12, 2016

@luixxiul - the routine you want to look at in app/ledger.js is synopsisNormalizer. look for this line

        if (pct[i] < 0) pct[i] = 0

underneath that line add these lines

        if (pct[i] <= minimumPercentage) {
          data = data.slice(0, i)
          break
        }

and that should do the trick... good luck!

@luixxiul
Copy link
Contributor

@mrose17 thanks! I will check it out ;-)

@luixxiul
Copy link
Contributor

I got ReferenceError: minimumPercentage is not defined error. I think it is too complex for me to solve this :-(

@mrose17
Copy link
Member

mrose17 commented Dec 13, 2016

@luixxiul - sorry, minimimumPercentage was a place-holder for the real variable. let's try this: tell me what repo you have your updates in and i'll update the code... thanks!

@luixxiul
Copy link
Contributor

@mrose17 appreciated! I will create a PR for it 👍

@luixxiul
Copy link
Contributor

@mrose17 I created the PR. Would you please update the code? Thanks!

mrose17 added a commit that referenced this issue Dec 14, 2016
@luixxiul luixxiul assigned luixxiul and unassigned diracdeltas Dec 14, 2016
@luixxiul luixxiul removed the help wanted The PR/issue opener needs help to complete/report the task. label Dec 14, 2016
@luixxiul luixxiul modified the milestones: 0.13.0, 1.1.0 Dec 14, 2016
@luixxiul luixxiul modified the milestones: 0.12.15, 0.13.0 Dec 14, 2016
@luixxiul
Copy link
Contributor

Merged in the master branch with this: 8ae8eb4

@luixxiul
Copy link
Contributor

luixxiul commented Dec 14, 2016

not merged in 0.12.15 build

@luixxiul
Copy link
Contributor

Test plan:

Open about:preferences#payments
Click Advanced settings
Toggle the switch
Make sure the sites with less than 1% usage are hidden

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

No branches or pull requests

6 participants