Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support publisher branded banner #1923

Closed
NejcZdovc opened this issue Oct 31, 2018 · 4 comments · Fixed by brave/brave-core#939
Closed

Support publisher branded banner #1923

NejcZdovc opened this issue Oct 31, 2018 · 4 comments · Fixed by brave/brave-core#939

Comments

@NejcZdovc
Copy link
Contributor

NejcZdovc commented Oct 31, 2018

Description

Currently we are not displaying images that are returned from the server for publishers in site banner.

Steps to Reproduce

  1. enable staging rewards
  2. go to https://yachtcaptain23.github.io
  3. click on bat logo and click tip

Actual result:

Site banner shows default banner

Expected result:

Site banner should show server image if that image exists

Reproduces how often:

Brave version (brave://version info)

Reproducible on current release:

  • Does it reproduce on brave-browser dev/beta builds?

Website problems only:

  • Does the issue resolve itself when disabling Brave Shields?
  • Is the issue reproducible on the latest version of Chrome?

Additional Information

Currently it's disabled here https://github.com/brave/brave-core/blob/master/components/brave_rewards/resources/donate/components/siteBanner.tsx#L116

We already get all necessary data from the server.

@NejcZdovc NejcZdovc added the priority/P3 The next thing for us to work on. It'll ride the trains. label Oct 31, 2018
@bbondy bbondy added this to the 1.x Backlog milestone Nov 5, 2018
@NejcZdovc NejcZdovc added priority/P2 A bad problem. We might uplift this to the next planned release. and removed priority/P3 The next thing for us to work on. It'll ride the trains. labels Nov 12, 2018
@NejcZdovc NejcZdovc changed the title Site banner serve images Support publisher branded banner Nov 19, 2018
@emerick emerick added the QA/Yes label Nov 21, 2018
@NejcZdovc NejcZdovc modified the milestones: 1.x Backlog, 0.59.x - Dev Dec 4, 2018
@NejcZdovc NejcZdovc modified the milestones: 0.59.x - Dev, 0.58.x - Beta Dec 4, 2018
@btlechowski
Copy link

btlechowski commented Dec 12, 2018

Verification passed on

Brave 0.58.11 Chromium: 71.0.3578.80 (Official Build) beta (64-bit)
Revision 2ac50e7249fbd55e6f517a28131605c9fb9fe897-refs/branch-heads/3578@{#860}
OS Windows 7 x64

Used STR from OP. Test staging server.

Pretty cool:
image

Verification PASSED on Ubuntu 18.04 x64 using the following build:

Brave 0.58.11 Chromium: 71.0.3578.80 (Official Build) beta (64-bit)
Revision 2ac50e7249fbd55e6f517a28131605c9fb9fe897-refs/branch-heads/3578@{#860}
OS Linux

screen shot 2018-12-12 at 1 57 46 am

Verified passed with

Brave 0.58.15 Chromium: 71.0.3578.98 (Official Build) (64-bit)
Revision 15234034d19b85dcd9a03b164ae89d04145d8368-refs/branch-heads/3578@{#897}
OS Mac OS X
  • have been testing with @kjozwiak - issues found on publishers side are being reported in https://github.com/brave-intl/creators-private-issues/issues/826
  • since browser only pulls this data once a day, we've been updating pubs_load_timestamp in publisher_state file (profile/default folder) to get latest from publishers. Note, need to wait some time after updating on publishers site since it's not instantaneous.
    screen shot 2018-12-19 at 8 48 59 am

@kjozwiak
Copy link
Member

Updated the banner for kjozwiak.github.io and waiting the required ~2hrs. Will update the above with verification comment with updated screens once the changes occur. 👍

@dlipeles
Copy link

Hi @NejcZdovc we recently added a new feature to the editor which allows publishers to select tip amounts from a list of presets.

@kjozwiak had some reports of the tips not persisting to the client-side, I believe we are serving them correctly from our /channels api

My guess is that there was some code added to restrict the values to [1,5,10] when Product team decided that a few weeks ago. I think the solution would be to open up the restriction to the following amounts

{ 1, 5, 10 }
{ 5, 10, 20}
{ 10, 20, 50 }
{ 20, 50, 100 }
{ 50, 100, 500 }

@NejcZdovc
Copy link
Contributor Author

@dlipeles created issue to implement it as it's currently hard coded #2645

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

Successfully merging a pull request may close this issue.

8 participants