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

Allow banner to show notice information #5125

Merged
merged 23 commits into from
Jul 30, 2024

Conversation

gilluminate
Copy link
Contributor

@gilluminate gilluminate commented Jul 23, 2024

Closes PROD-2427 and PROD-2386

Description Of Changes

Update experience form with option for including notices in the banner & update banner to accommodate.

Code Changes

  • Added support for new experience config props show_layer1_notices and layer1_button_options
  • Added notices toggle in UI of Privacy Experience form and dropdown to select Acknowledge button.
  • Added list of notices in FidesJS if toggle is on, as well as display/save correct button if Acknowledge button is selected.
  • Update rules for calling notices served API
    --
  • Fixed unrelated UI issue where the delete button for Notices wasn't able to be clicked because of layout issues
  • Fixed unrelated failing FF Cypress test due to realClick being unavailable for that browser

CleanShot 2024-07-30 at 12 16 20@2x


CleanShot 2024-07-30 at 12 17 16@2x


Steps to Confirm

  • Visit Privacy Experiences in Admin UI
  • Click an experience that has banners
  • Add Notices if there aren't any on the experience
  • toggle the new switch "Allow banner to show notice information"
  • Notice the Preview window shows the notices in a list on the banner
  • Click to open Banner Options menu
  • Select Acknowledge option
  • Notice the Preview window shows the acknowledge button in place of Opt In/Opt Out buttons
  • NOTE: saving will not work until related BE is done.

Pre-Merge Checklist

  • All CI Pipelines Succeeded
  • Documentation:
    • documentation complete, PR opened in fidesdocs
    • documentation issue created in fidesdocs
    • if there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
  • Issue Requirements are Met
  • Relevant Follow-Up Issues Created
  • Update CHANGELOG.md

Copy link

vercel bot commented Jul 23, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
fides-plus-nightly ⬜️ Ignored (Inspect) Visit Preview Jul 30, 2024 10:20pm

Copy link

cypress bot commented Jul 23, 2024



Test summary

4 0 0 0


Run details

Project fides
Status Passed
Commit 559a87a2a0 ℹ️
Started Jul 30, 2024 10:32 PM
Ended Jul 30, 2024 10:33 PM
Duration 00:37 💡
OS Linux Ubuntu -
Browser Electron 106

View run in Cypress Cloud ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Cloud

@gilluminate gilluminate force-pushed the gill/PROD-2427_fe-update-experience-form branch from df36be7 to 8b44338 Compare July 30, 2024 20:12
@gilluminate gilluminate marked this pull request as ready for review July 30, 2024 20:19
Copy link
Contributor

@eastandwestwind eastandwestwind left a comment

Choose a reason for hiding this comment

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

First pass code-only review- just some small things.

The most risky thing I want to make sure of is the CSS changes- let's just eval that what we change with existing classes / DOM elements won't break current customers, or is at least low-risk

Copy link

codecov bot commented Jul 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.54%. Comparing base (a318829) to head (d90d820).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5125   +/-   ##
=======================================
  Coverage   86.54%   86.54%           
=======================================
  Files         357      357           
  Lines       22330    22331    +1     
  Branches     2955     2955           
=======================================
+ Hits        19326    19327    +1     
  Misses       2480     2480           
  Partials      524      524           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

* This React hook listens for matches to a CSS media query. It allows the rendering of components based on whether the query matches or not. The media query string can be any valid CSS media query, for example '(prefers-color-scheme: dark)'.
* @param query - The CSS media query string to match against the viewport.
* @example const isMobile = useMediaQuery('(max-width: 768px)');
* @returns A boolean value indicating whether the media query matches the current viewport.
Copy link
Contributor

Choose a reason for hiding this comment

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

💯

Copy link
Contributor

@eastandwestwind eastandwestwind left a comment

Choose a reason for hiding this comment

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

Ran through manual testing matrix via screenshare, everything is working as expected!

@gilluminate gilluminate merged commit 59de170 into main Jul 30, 2024
51 checks passed
@gilluminate gilluminate deleted the gill/PROD-2427_fe-update-experience-form branch July 30, 2024 23:01
Copy link

cypress bot commented Jul 30, 2024



Test summary

4 0 0 0


Run details

Project fides
Status Passed
Commit 59de170
Started Jul 30, 2024 11:14 PM
Ended Jul 30, 2024 11:15 PM
Duration 00:35 💡
OS Linux Ubuntu -
Browser Electron 106

View run in Cypress Cloud ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Cloud

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.

2 participants