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

Notice only rendering #3546

Merged
merged 15 commits into from
Jun 14, 2023
Merged

Notice only rendering #3546

merged 15 commits into from
Jun 14, 2023

Conversation

allisonking
Copy link
Contributor

@allisonking allisonking commented Jun 12, 2023

Closes #3443

Code Changes

  • Conditionally render acknowledge button in the banner
  • Conditionally render acknowledge button in the modal
  • Make notice-only notices disabled and set to true in both fides-js and privacy center
  • Update tests
  • Refactored the button component out of the banner and modal since they have similar logic. A few style tweaks ensued. It also cuts down on our bundle size by about ~.1kb now that we are using more shared components and styles!
    • This diff is pretty big, but is mostly contained to this commit 6dfec80

Steps to Confirm

  • Edit your fides-js-demo-components.html such that both notices are notice_only and have a default preference of opt_in (which all notice_only notices should have)
  • turbo dev in the fides-js folder and visit http://localhost:3000/fides-js-components-demo.html
  • You should see the banner render "OK" instead of accept/reject
  • Click the modal link to view the notice-only notices which can't be toggled off. You should also see "OK" here instead of the 3 buttons
  • Clicking "OK" should send an acknowledge preference
  • Visit localhost:3000. You'll need to populate your notices with notice only ones. You should see the radio button becomes disabled

Pre-Merge Checklist

Description Of Changes

image image

When only one notice is notice-only
image

Privacy center if we make a notice notice-only
image

@allisonking allisonking force-pushed the aking/3443/notice-only-rendering branch from ef4f77f to b87d7aa Compare June 12, 2023 18:55
@cypress
Copy link

cypress bot commented Jun 12, 2023

Passing run #2658 ↗︎

0 4 0 0 Flakiness 0
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.

Details:

Merge 3516eb5 into 0626375...
Project: fides Commit: 796a4c70c1 ℹ️
Status: Passed Duration: 00:49 💡
Started: Jun 13, 2023 6:51 PM Ended: Jun 13, 2023 6:52 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@allisonking allisonking marked this pull request as ready for review June 13, 2023 18:43
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.

looks great, thanks for adding e2e tests too!

@allisonking allisonking merged commit 225f211 into main Jun 14, 2023
@allisonking allisonking deleted the aking/3443/notice-only-rendering branch June 14, 2023 15:12
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.

Render acknowledge button when there are only notice_only notices
2 participants