-
Notifications
You must be signed in to change notification settings - Fork 74
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
Mobile Native POC support work #5541
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
fides Run #11249
Run Properties:
|
Project |
fides
|
Branch Review |
refs/pull/5541/merge
|
Run status |
Passed #11249
|
Run duration | 00m 49s |
Commit |
f9208df1c9 ℹ️: Merge 3ac980db916b7dbfde342ef8d10ca307419e8f97 into a601fbeeaece3ce7fe6c48a577ac...
|
Committer | Jason Gill |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
4
|
View all changes introduced in this branch ↗︎ |
426c328
to
3af1ba8
Compare
8d650fb
to
7e5a316
Compare
7e5a316
to
860925f
Compare
onSave: () => { | ||
setBannerIsOpen(false); | ||
}, |
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.
onSave
was just superfluous and confusing so I removed it in favor of onClose
everywhere.
fidesDebugger( | ||
`GPP: CMP status and configuration for ${geolocation?.location}`, | ||
{ | ||
cmpStatus: cmpApi.getCmpStatus(), | ||
cmpDisplayStatus: cmpApi.getCmpDisplayStatus(), | ||
signalStatus: cmpApi.getSignalStatus(), | ||
supportedAPIs: cmpApi.getSupportedAPIs(), | ||
applicableSections: cmpApi.getApplicableSections(), | ||
gppString: cmpApi.getGppString(), | ||
parsedSections: cmpApi.getObject(), | ||
}, | ||
); |
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.
a minor wish-list item I'm sneaking in to this PR :)
@@ -89,6 +89,221 @@ | |||
"geolocation": { | |||
"location": "eea", | |||
"country": "eea" | |||
}, | |||
"gvl": { |
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.
automated preference saving was throwing an error because this was missing from the minimal experience mocks
<Component {...pageProps} /> | ||
); | ||
const App = ({ Component, pageProps }: AppProps) => { | ||
const AnyComponent = Component as any; |
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.
weird solution to a weird Typescript problem with NextJS types.
@@ -229,6 +229,47 @@ const NoticeOverlay: FunctionComponent<OverlayProps> = ({ | |||
], | |||
); | |||
|
|||
const handleAcceptAll = useCallback( |
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.
bumped these up to parent to take advantage of them at this level for the automated preference
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.
Looking good so far @gilluminate ! Nice work implementing for not only notice-based but also TCF version as well.
renderFirstButton={() => ( | ||
<Button | ||
buttonType={ButtonType.SECONDARY} | ||
label={i18n.t("exp.save_button_label")} | ||
onClick={() => onSave(ConsentMethod.SAVE, draftIds!)} | ||
onClick={() => onSave(ConsentMethod.SAVE, draftIds)} |
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.
what did the !
actually do before?
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.
Its purpose is to tell the Typescript validator to ignore the fact that it might be null/undefined whereas (in this case) the onSave
method expects it to have a value. Usually reserved for something where the developer is 100% sure that value will be set by the time it hits this chunk of code, I find it's usually abused and prone to unexpected bugs so I prefer cleaning it up wherever possible. In this case I set the value explicitly on line 174, giving the useState
a default.
fides Run #11250
Run Properties:
|
Project |
fides
|
Branch Review |
main
|
Run status |
Passed #11250
|
Run duration | 00m 55s |
Commit |
8723d8468f: Mobile Native POC support work (#5541)
|
Committer | Jason Gill |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
4
|
View all changes introduced in this branch ↗︎ |
Closes HJ-259
Description Of Changes
Includes prep-work needed to be done in Fides before the native POC can work as intended
fides_consent_override
which will automatically set the user’s consent preference.script
consent method to distinguish between manual and automated consent.Steps to Confirm
BEFORE:
AFTER:
Pre-Merge Checklist
CHANGELOG.md
updatedmain
downgrade()
migration is correct and works