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

Fixing incorrect verified notice behavior in tipping banner #3552

Merged
merged 1 commit into from
Oct 1, 2019
Merged

Conversation

ryanml
Copy link
Contributor

@ryanml ryanml commented Sep 26, 2019

Fixes: brave/brave-browser#6182

Screen Shot 2019-09-26 at 4 13 00 PM

Submitter Checklist:

Test Plan:

Defined in issue

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

@ryanml ryanml added this to the 0.72.x - Nightly milestone Sep 26, 2019
@ryanml ryanml requested a review from a team September 26, 2019 23:15
@ryanml ryanml self-assigned this Sep 26, 2019
@ryanml ryanml force-pushed the fix-6182 branch 2 times, most recently from 2752d43 to 5e2d50b Compare September 30, 2019 08:08
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.

there is one use case that doesn't work.

  1. clean profile (staging)
  2. connect to uphold wallet that is not KYC
  3. visit https://kjozwiakstaging.github.io
  4. open panel (see the message)
  5. tip and you don't see a message

@ryanml ryanml merged commit d5f0890 into master Oct 1, 2019
@ryanml ryanml deleted the fix-6182 branch October 1, 2019 04:51
ryanml added a commit that referenced this pull request Oct 1, 2019
Fixing incorrect verified notice behavior in tipping banner
ryanml added a commit that referenced this pull request Oct 1, 2019
Fixing incorrect verified notice behavior in tipping banner
@kjozwiak
Copy link
Member

kjozwiak commented Oct 1, 2019

@mandar-brave @NejcZdovc @ryanml is there a reason this needs to be uplifted into 0.71.x? I understand it's a regression but we're trying to limit uplifts for issues that are either critical or greatly reduce functionality.

@mandar-brave
Copy link

@kjozwiak basis its the first step in the process and being upfront is good. Step-1 is user tapping the BAT logo. I would prefer to keep it for this release knowing it is a 0.69 specific change.

@kjozwiak
Copy link
Member

kjozwiak commented Oct 1, 2019

@mandar-brave if we want this in 0.69.x, we'll need another PR for 0.69.x as the following were the only ones creates:

At this point, we're cutting it pretty close here and landing something basically last minute.

@NejcZdovc
Copy link
Contributor

@kjozwiak yeah because of close I only asked @ryanml to do 71 and 70. @mandar-brave let us know what to do

@mandar-brave
Copy link

@kjozwiak i understand the constraints so you make the final call on risk (it is not a blocker at my end but definitely prefer to see it in this release). I am responding from a product perspective. considering the text is newly introduced specifically for 0.69, it would definitely leave inconsistencies from the previous experience (where the user does not have to click send-tip before text to suggest tip may not be sent).

@rebron
Copy link
Collaborator

rebron commented Oct 1, 2019

Not a blocker and we're too late on 69.x for the fix. Fix will be in 70.x though.

fmarier pushed a commit that referenced this pull request Oct 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tip banner has message for connected publisher when panel does not - follow up to 5925
5 participants