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

feat: add a dismissible conversion change warning #1249

Merged
merged 2 commits into from
Jul 8, 2024

Conversation

IanKrieger
Copy link
Member

@IanKrieger IanKrieger commented Jul 3, 2024

Lets the end user know conversions have stricter enforcement:

Screen Shot 2024-07-03 at 1 44 18 PM
Screen Shot 2024-07-03 at 2 02 00 PM

@IanKrieger IanKrieger requested a review from a team as a code owner July 3, 2024 18:06
@lukaslevert
Copy link
Collaborator

Sharing my feedback from Slack, but it's not a blocker. I'd prefer to display the link more cleanly as one of these:

Ads Help Center
ads-help.brave.com

Copy link

github-actions bot commented Jul 8, 2024

[puLL-Merge] - brave/ads-ui@1249

Description

This PR makes significant changes to the conversion tracking functionality in the Brave Ads UI. It simplifies the conversion setup process, updates the user interface, and adds a new alert about changes to conversion reporting.

Changes

Changes

  1. src/components/Conversion/ConversionDisplay.tsx:

    • Removed "Conversion Type" field
    • Reordered fields to show URL Pattern first
    • Changed "Observation Window" to "Conversion Observation Window"
  2. src/components/Conversion/ConversionFields.tsx:

    • Removed radio control for conversion type
    • Updated helper text for observation window
    • Added LearnMoreButton to observation window field
  3. src/locales/*.po:

    • Updated translations for various languages to reflect changes in the UI
  4. src/user/views/adsManager/types/index.ts:

    • Updated initial conversion values: type is now "postclick" by default, observation window is 30 days
  5. src/user/views/adsManager/views/advanced/components/adSet/fields/ConversionField.tsx:

    • Updated helper text for URL input
    • Changed UI for adding/removing conversion tracking
    • Replaced text link with a button for removing conversion tracking
  6. src/user/views/user/CampaignView.tsx:

    • Added ConversionAlert component to the campaign view
  7. src/user/views/user/ConversionAlert.tsx:

    • New file: Added a new alert component to inform users about changes in conversion reporting

Possible Issues

  1. The removal of the conversion type selection (post-view vs post-click) might affect existing campaigns or reporting. It's unclear how this change will be handled for existing data.

  2. The default 30-day observation window might not be suitable for all advertisers and could lead to confusion if not clearly communicated.

Security Hotspots

No significant security issues were identified in this change. However, ensure that the new ConversionAlert component doesn't expose any sensitive information and that the link to the help center is correctly implemented to prevent potential phishing attempts.

@IanKrieger IanKrieger merged commit 5f951ca into master Jul 8, 2024
8 checks passed
@IanKrieger IanKrieger deleted the feat/improve-conversion-config branch July 8, 2024 14:30
tackley added a commit that referenced this pull request Aug 1, 2024
This was introduced nearly a month ago in #1249. It can now be removed as all that need to have seen it will have done so.
IanKrieger pushed a commit that referenced this pull request Aug 1, 2024
This was introduced nearly a month ago in #1249. It can now be removed
as all that need to have seen it will have done so.
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.

3 participants