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

Adds confirmation dialog upon clicking the trash icon in the ledger table #13182

Merged
merged 1 commit into from
Feb 19, 2018

Conversation

ryanml
Copy link
Contributor

@ryanml ryanml commented Feb 19, 2018

Fixes Issue #11164

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.
  • Request a security/privacy review as needed. (Ask a Brave employee to help if you cannot access this document.)

Test Plan:

  1. Visit some sites
  2. Open about:payments
  3. Click a trash icon
  4. Ensure that the appearing confirmation dialogue, depending on the choice, allows the removal of the site or does nothing.

Reviewer Checklist:

  • Request a security/privacy review as needed if one was not already requested.

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

Two translations added under the name: banSiteConfirmation. Mechanism for creating the confirmation dialogue mimics that shown in the syncTable component. Ref: here

@ryanml
Copy link
Contributor Author

ryanml commented Feb 19, 2018

@NejcZdovc seems I royally screwed up the squash ;( I figured just opening up another PR in this case would be cleaner.. We should be good to go :)

@codecov-io
Copy link

codecov-io commented Feb 19, 2018

Codecov Report

Merging #13182 into master will decrease coverage by <.01%.
The diff coverage is 20%.

@@            Coverage Diff             @@
##           master   #13182      +/-   ##
==========================================
- Coverage   56.18%   56.17%   -0.01%     
==========================================
  Files         279      279              
  Lines       27873    27877       +4     
  Branches     4560     4561       +1     
==========================================
+ Hits        15660    15661       +1     
- Misses      12213    12216       +3
Flag Coverage Δ
#unittest 56.17% <20%> (-0.01%) ⬇️
Impacted Files Coverage Δ
app/locale.js 35.77% <ø> (ø) ⬆️
...erer/components/preferences/payment/ledgerTable.js 86.5% <20%> (-1.26%) ⬇️

@NejcZdovc NejcZdovc added this to the 0.23.x (Nightly Channel) milestone Feb 19, 2018
@NejcZdovc NejcZdovc self-requested a review February 19, 2018 06:34
Copy link
Contributor

@NejcZdovc NejcZdovc left a comment

Choose a reason for hiding this comment

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

++ looks good to me. Thank you for this one 🎉

@NejcZdovc NejcZdovc merged commit b621fd3 into brave:master Feb 19, 2018
@ryanml
Copy link
Contributor Author

ryanml commented Feb 19, 2018

Thanks @NejcZdovc ! 👍

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.

3 participants