Skip to content

Conversation

@shane-landry
Copy link

@shane-landry shane-landry commented Nov 5, 2020

Background

  • Adds new slim banner with feature flag
  • Adds full width mode with feature flag
  • Adds drop shadow toggle feature flag
  • Adds close with × buttom feature flag
  • Adds default background color override

Test Plan

yarn dev

Go to http://localhost:8080/tcf-2.0.html

  1. Layer0 - Check "Use Slim"
  • Slim banner (layer 0) should load.
  • "Learn More" Should navigate to Layer1.
  • "Ok, Got it" should accept all
  • Test layout on mobile
  1. Full Width Mode - Check "Full Width"
  • Layer1 and Vendor list should span full width
  • No rounded corners on Layer1 and Vendor list
  1. Toggle Drop Shadow - (Un)Check "Drop Shadow"
  • When checked, all layers have drop shadow
  • When not checked, no layers have drop shadow
  1. Close (×) button - Check "Close with ×"
  • When checked, all banner layers should have × button displayed in upper right
  • When unchecked, not button should render
  • Clicking should "accept all" and close CMP UI
  • Check positioning on mobile UI
  1. Background color override - Set config option theme: { backgroundColor: "#f1f1f1" }
  • Banner background colors should display configured color
  • No overlapping or nested UI elements should overlay with another background color

@shane-landry shane-landry requested a review from potench November 5, 2020 20:33
maxHeight: maxHeightModal,
}}
>
{ showCloseX && <div class={style.closeX} onClick={this.handleAcceptAll}>&times;</div>}
Copy link

Choose a reason for hiding this comment

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

checking with product on this, think compliance didn't want close button to accept all originally but that mighta changed

maxHeight: maxHeightModal,
}}
>
{ shouldShowCloseX && <div class={style.closeX} onClick={this.handleAcceptAll}>&times;</div>}
Copy link

Choose a reason for hiding this comment

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

can you change this so we can track when user clicks "Close" vs "acceptAll" will be good to see telemetry on that.

onClick={this.handleClose}
//  ....

// further up 
handleClose = () => {
    this.handleAcceptAll('acceptAllClose');
}

handleAcceptAll = (clickCategoty = 'acceptAll') => {
 // change logger click category 
};

Copy link
Author

Choose a reason for hiding this comment

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

Added these to the 3 banners.

Copy link

@potench potench left a comment

Choose a reason for hiding this comment

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

LGTM

@shane-landry shane-landry merged commit 107fe8d into master Nov 16, 2020
@shane-landry shane-landry deleted the PFE-4216 branch November 16, 2020 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants