-
Notifications
You must be signed in to change notification settings - Fork 72
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
Apply any updated state to the overlay #5384
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
fides Run #10464
Run Properties:
|
Project |
fides
|
Run status |
Passed #10464
|
Run duration | 00m 40s |
Commit |
9f4160271f ℹ️: Merge ac46a987b0f91d2503aa16adfed81edc28e16c03 into 46cc4155f7e2c12fe871ef60069f...
|
Committer | Jason Gill |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
4
|
Upgrade your plan to view test results. |
a86b4be
to
c52a4c3
Compare
c52a4c3
to
ed40fda
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just a minor naming thing. Consent state management overall in Fides-js needs a re-work imo, to find a source of truth between cookie / api / event obj / Fides window obj consent states 😬
CHANGELOG.md
Outdated
@@ -20,6 +20,7 @@ The types of changes are: | |||
### Fixed | |||
- Fixed a bug where D&D tables were rendering stale data [#5372](https://github.com/ethyca/fides/pull/5372) | |||
- Fixed issue where Dataset with nested fields was unable to edit Categories [#5383](https://github.com/ethyca/fides/pull/5383) | |||
- Fixed racecondition where GPP being updated after FidesJS initialization caused Privacy Notices to be in the wrong state [#5384](https://github.com/ethyca/fides/pull/5384) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you mean GPC
here?
@@ -124,6 +125,11 @@ const NoticeOverlay: FunctionComponent<OverlayProps> = ({ | |||
Array<string> | |||
>(initialEnabledNoticeKeys()); | |||
|
|||
window.addEventListener("FidesUpdating", (event) => { | |||
// If GPP is being applied after initialization, we need to update the initial overlay to reflect the new state. This is especially important for Firefox browsers (Gecko) because GPP gets applied rather late due to how it handles queuing the `setTimeout` on the last step of our `initialize` function. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here GPC
fides Run #10484
Run Properties:
|
Project |
fides
|
Run status |
Passed #10484
|
Run duration | 00m 38s |
Commit |
88a32f00b5: Apply any updated state to the overlay (#5384)
|
Committer | Jason Gill |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
4
|
Upgrade your plan to view test results. |
Closes HJ-22
Description Of Changes
If GPC (browser's "Do Not Track") is being applied after Fides initialization, we need to update the initial overlay to reflect the new state.
This is especially important for Firefox browsers (Gecko) because—not only does FF make it easy to enable GPC—GPC gets applied rather late due to how Gecko handles queuing the
setTimeout
on the last step of ourinitialize
function.Code Changes
FidesUpdating
and use that to update the Overlay's notices as applicableSteps to Confirm
Pre-Merge Checklist
CHANGELOG.md