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

Formatting for Saved Site Exceptions (deleted YT channels) should match ledger table #12271

Closed
LaurenWags opened this issue Dec 13, 2017 · 7 comments
Assignees
Labels

Comments

@LaurenWags
Copy link
Member

LaurenWags commented Dec 13, 2017

Test Plan

See STR.
Also need to check ledger entries which were deleted in previous versions of Brave.

Description

After deleting a YT channel from your ledger table, if you go to re-add it under Shields > Saved Site Exceptions > Brave Payments, the YouTube channel is listed, the formatting doesn't match how the payments screen displays it.

Steps to Reproduce

  1. Visit a YT channel for min time so it displays in your ledger table
  2. Click the trash can icon to remove from ledger table
  3. Navigate to Preferences > Payments > Show Deleted sites

Actual result:
Formatting for site name doesn't display the way it does on ledger:
screen shot 2017-12-13 at 8 55 57 am

Expected result:
Formatting should match ledger for readability and in case user had accidentally removes one.

Reproduces how often:
Easily

Brave Version

about:brave info:
Brave | 0.19.115
rev | 584e694
Muon | 4.5.25

Reproducible on current live release:
yes

Additional Information

Similar to #12217

@LaurenWags LaurenWags added bug feature/rewards priority/P4 Minor loss of function. Workaround usually present. labels Dec 13, 2017
@LaurenWags LaurenWags added this to the Backlog (Prioritized) milestone Dec 13, 2017
@NejcZdovc NejcZdovc self-assigned this Dec 15, 2017
@NejcZdovc
Copy link
Contributor

do you think that we need to update old (existing) entries as well or only new ones?

NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Dec 15, 2017
@LaurenWags
Copy link
Member Author

@NejcZdovc I think it would be great if we could update the existing entries. WDYT @kjozwiak @srirambv @bsclifton ?

@srirambv
Copy link
Collaborator

Agreed would be good to have the existing entries updated.

@NejcZdovc NejcZdovc modified the milestones: Backlog (Prioritized), 0.19.x Hotfix 9 Dec 16, 2017
@bsclifton bsclifton modified the milestones: 0.19.x Hotfix 9, 0.21.x (Developer Channel) Dec 20, 2017
@NejcZdovc NejcZdovc modified the milestones: 0.21.x (Developer Channel), 0.20.x Hotfix 1 Jan 16, 2018
@alexwykoff alexwykoff modified the milestones: 0.20.x Hotfix 3 (Ledger improvments), 0.21.x (Beta Channel) Feb 6, 2018
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Feb 16, 2018
@alexwykoff alexwykoff added priority/P5 Cosmetic. Spelling, copy, layout. New features (which should also be part of an initiative). and removed priority/P4 Minor loss of function. Workaround usually present. labels Feb 27, 2018
@alexwykoff alexwykoff modified the milestones: 0.22.x (Developer Channel), Backlog (Prioritized) Feb 27, 2018
@bsclifton bsclifton modified the milestones: Backlog (Prioritized), Completed work Feb 28, 2018
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Mar 14, 2018
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Mar 14, 2018
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Mar 14, 2018
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Apr 10, 2018
@NejcZdovc NejcZdovc modified the milestones: Completed work, 0.22.x Release 2, 0.22.x Release 3 (Beta channel) Apr 10, 2018
NejcZdovc added a commit that referenced this issue Apr 11, 2018
Fixes formatting for YT on shields page
NejcZdovc added a commit that referenced this issue Apr 11, 2018
Fixes formatting for YT on shields page
NejcZdovc added a commit that referenced this issue Apr 11, 2018
Fixes formatting for YT on shields page
@srirambv
Copy link
Collaborator

srirambv commented May 3, 2018

image
Verified on Windows x64

  • 0.22.702 e4a853d
  • libchromiumcontent 66.0.3359.139
  • muon: 6.0.7

Verified with macOS 10.12.6 using

Verified on Ubuntu 17.10 x64

  • 0.22.703 903b8d0
  • libchromiumcontent 66.0.3359.139
  • muon: 6.0.8

@LaurenWags
Copy link
Member Author

LaurenWags commented May 3, 2018

@NejcZdovc if a YouTube or Twitch site was deleted in 0.22.669 and you update to 0.22.702, it displays as below. While this is more read-able than previously, it doesn't match what happens if you delete a site using 0.22.702 (see screenshot from #12271 (comment) ). Expected?
screen shot 2018-05-03 at 5 15 12 pm

@NejcZdovc
Copy link
Contributor

@LaurenWags we set version value a long time ago and it was set to trigger upgrade if version is smaller then 0.22.3. We can adjust this value to 0.22.7.

@NejcZdovc
Copy link
Contributor

Just did some more testing and with the latest code that we have for 0.22 we can't preform upgrade for this case anymore, because we don't have information about the name. So this scenario is expected.

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

7 participants