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.

Sign up for GitHub

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

TCF experience design improvements #4222

Merged
merged 18 commits into from
Oct 10, 2023
Merged

TCF experience design improvements #4222

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
c13775c
First pass
galvana Oct 3, 2023
2057c1f
Merge branch 'main' into PROD-1084-TCF-experience-design-improvements
galvana Oct 3, 2023
ad84405
Additional styling
galvana Oct 5, 2023
0be3aea
Matching more of the designs
galvana Oct 5, 2023
ae8fb9c
Adding class to banner button group
galvana Oct 6, 2023
1c5badd
Adding middle button prop to button group
galvana Oct 6, 2023
ff59616
Preventing modal from closing when clicking outside
galvana Oct 6, 2023
6830081
Merge branch 'main' into PROD-1084-TCF-experience-design-improvements
galvana Oct 6, 2023
d33f8ce
Lint fixes
galvana Oct 6, 2023
baf9545
Running prettier
galvana Oct 6, 2023
25211f2
Differentiating between banner and modal title fonts
galvana Oct 6, 2023
32aa418
Changes based on PR feedback
galvana Oct 6, 2023
e689a4c
Merging in main
galvana Oct 9, 2023
528788b
Moving all on off buttons
galvana Oct 9, 2023
2803482
Running prettier
galvana Oct 9, 2023
db39bfe
One more styling change
galvana Oct 9, 2023
b208e11
Updating change log and docstring
galvana Oct 10, 2023
0d66d64
Merge branch 'main' into PROD-1084-TCF-experience-design-improvements
galvana Oct 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clients/fides-js/rollup.config.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import postcss from "rollup-plugin-postcss";

const NAME = "fides";
const IS_DEV = process.env.NODE_ENV === "development";
const GZIP_SIZE_ERROR_KB = 20; // fail build if bundle size exceeds this
const GZIP_SIZE_ERROR_KB = 22; // fail build if bundle size exceeds this
galvana marked this conversation as resolved.
Show resolved Hide resolved
const GZIP_SIZE_WARN_KB = 15; // log a warning if bundle size exceeds this

// TCF
Expand Down
60 changes: 40 additions & 20 deletions clients/fides-js/src/components/ConsentBanner.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import { h, FunctionComponent, ComponentChildren } from "preact";
import {
h,
FunctionComponent,
ComponentChildren,
Component,
VNode,
} from "preact";
import { getConsentContext } from "../lib/consent-context";
import { ExperienceConfig } from "../lib/consent-types";
import CloseButton from "./CloseButton";
Expand All @@ -9,8 +15,10 @@ interface BannerProps {
experience: ExperienceConfig;
onClose: () => void;
bannerIsOpen: boolean;
children: ComponentChildren;
children?: ComponentChildren;
onVendorPageClick?: () => void;
allisonking marked this conversation as resolved.
Show resolved Hide resolved
buttonGroup: VNode;
bannerType?: String;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't even know there was a String! looks like we should prefer string tho: https://stackoverflow.com/questions/14727044/what-is-the-difference-between-types-string-and-string

Suggested change
bannerType?: String;
bannerType?: string;

Copy link
Contributor

Choose a reason for hiding this comment

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

or even better, would be to restrict this to the strings it can be, i.e. "TCF"|"notices"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This ended up going away after refactoring the GPC check


const ConsentBanner: FunctionComponent<BannerProps> = ({
Expand All @@ -19,6 +27,8 @@ const ConsentBanner: FunctionComponent<BannerProps> = ({
bannerIsOpen,
children,
onVendorPageClick,
buttonGroup,
bannerType,
}) => {
const showGpcBadge = getConsentContext().globalPrivacyControl;
return (
Expand All @@ -31,27 +41,37 @@ const ConsentBanner: FunctionComponent<BannerProps> = ({
<div id="fides-banner">
<div id="fides-banner-inner">
<CloseButton ariaLabel="Close banner" onClick={onClose} />
<div id="fides-banner-heading">
<div id="fides-banner-title" className="fides-banner-title">
{experience.title}
</div>
{showGpcBadge ? (
<GpcBadge
label="Global Privacy Control Signal"
status="detected"
/>
) : null}
</div>
<div
id="fides-banner-description"
className="fides-banner-description"
id="fides-banner-inner-container"
style={{
gridTemplateColumns: children ? "1fr 1fr" : "1fr",
}}
galvana marked this conversation as resolved.
Show resolved Hide resolved
>
<ExperienceDescription
description={experience.description}
onVendorPageClick={onVendorPageClick}
/>
<div id="fides-banner-inner-description">
<div id="fides-banner-heading">
<div id="fides-banner-title" className="fides-banner-title">
{experience.title}
</div>
{showGpcBadge && bannerType !== "TCF" ? (
<GpcBadge
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Passing in bannerType to hide the GpcBadge for the TCF banner/modal

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm if its only purpose is whether or not to show the gpc badge, maybe instead of bannerType we make the prop hideGpc, then change above. I think that makes it a bit clearer for someone using this component exactly what that prop is going to do

const showGpcBadge = getConsentContext().globalPrivacyControl && !hideGpc;

Copy link
Contributor

Choose a reason for hiding this comment

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

actually maybe a better idea is to modify the function getGlobalPrivacyControl to query window.Fides.options.tcfEnabled. if it's enabled, return false. then I don't think gpc would ever render for TCF, and it'd be tied to gpc logic, as opposed to component logic, which is nice!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like that, much cleaner!

label="Global Privacy Control Signal"
status="detected"
/>
) : null}
</div>
<div
id="fides-banner-description"
className="fides-banner-description"
>
<ExperienceDescription
description={experience.description}
onVendorPageClick={onVendorPageClick}
/>
</div>
</div>
{children}
{buttonGroup}
</div>
{children}
</div>
</div>
</div>
Expand Down
25 changes: 21 additions & 4 deletions clients/fides-js/src/components/ConsentButtons.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { h, VNode } from "preact";
import { ComponentChildren, h, VNode } from "preact";
import Button from "./Button";
import {
ButtonType,
Expand All @@ -12,26 +12,35 @@ export const ConsentButtons = ({
experienceConfig,
onManagePreferencesClick,
firstButton,
middleButton,
onAcceptAll,
onRejectAll,
children,
}: {
experienceConfig: ExperienceConfig;
onManagePreferencesClick?: () => void;
firstButton?: VNode;
middleButton?: VNode;
onAcceptAll: () => void;
onRejectAll: () => void;
children: ComponentChildren;
}) => (
<div id="fides-button-group">
{onManagePreferencesClick ? (
<div>
<div style={{ display: "flex" }}>
<Button
buttonType={ButtonType.TERTIARY}
label={experienceConfig.privacy_preferences_link_label}
onClick={onManagePreferencesClick}
/>
</div>
) : null}
<div className={firstButton ? "fides-modal-button-group" : undefined}>
{middleButton || null}
<div
className={
firstButton ? "fides-modal-button-group" : "fides-banner-button-group"
}
>
{firstButton || null}
<Button
buttonType={ButtonType.PRIMARY}
Expand All @@ -44,6 +53,7 @@ export const ConsentButtons = ({
onClick={onAcceptAll}
/>
</div>
{children}
</div>
);

Expand All @@ -56,6 +66,8 @@ interface NoticeConsentButtonProps {
enabledKeys: NoticeKeys;
isAcknowledge: boolean;
isInModal?: boolean;
children?: ComponentChildren;
middleButton?: VNode;
allisonking marked this conversation as resolved.
Show resolved Hide resolved
}

export const NoticeConsentButtons = ({
Expand All @@ -65,6 +77,8 @@ export const NoticeConsentButtons = ({
enabledKeys,
isInModal,
isAcknowledge,
children,
middleButton,
}: NoticeConsentButtonProps) => {
if (!experience.experience_config || !experience.privacy_notices) {
return null;
Expand Down Expand Up @@ -118,6 +132,9 @@ export const NoticeConsentButtons = ({
/>
) : undefined
}
/>
middleButton={middleButton}
>
{children}
</ConsentButtons>
);
};
17 changes: 4 additions & 13 deletions clients/fides-js/src/components/ConsentModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ComponentChildren, h } from "preact";
import { ComponentChildren, VNode, h } from "preact";
import { Attributes } from "../lib/a11y-dialog";
import { ExperienceConfig } from "../lib/consent-types";

Expand All @@ -11,11 +11,13 @@ const ConsentModal = ({
experience,
children,
onVendorPageClick,
modalType,
}: {
attributes: Attributes;
experience: ExperienceConfig;
children: ComponentChildren;
onVendorPageClick?: () => void;
modalType?: String;
}) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

same thing here—see my meandering comment thoughts about on bannerType

const { container, overlay, dialog, title, closeButton } = attributes;

Expand Down Expand Up @@ -49,19 +51,8 @@ const ConsentModal = ({
description={experience.description}
/>
</p>
<GpcInfo />
{modalType !== "TCF" && <GpcInfo />}
{children}
{experience.privacy_policy_link_label &&
experience.privacy_policy_url ? (
allisonking marked this conversation as resolved.
Show resolved Hide resolved
<a
href={experience.privacy_policy_url}
rel="noopener noreferrer"
target="_blank"
className="fides-modal-privacy-policy"
>
{experience.privacy_policy_link_label}
</a>
) : null}
</div>
</div>
);
Expand Down
5 changes: 4 additions & 1 deletion clients/fides-js/src/components/Overlay.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ interface Props {
renderBanner: (props: RenderBannerProps) => VNode | null;
renderModalContent: (props: RenderModalContent) => VNode;
onVendorPageClick?: () => void;
overlayType?: String;
}

const Overlay: FunctionComponent<Props> = ({
Expand All @@ -42,6 +43,7 @@ const Overlay: FunctionComponent<Props> = ({
renderBanner,
renderModalContent,
onVendorPageClick,
overlayType,
}) => {
const delayBannerMilliseconds = 100;
const delayModalLinkMilliseconds = 200;
Expand All @@ -54,7 +56,7 @@ const Overlay: FunctionComponent<Props> = ({

const { instance, attributes } = useA11yDialog({
id: "fides-modal",
role: "dialog",
role: "alertdialog",
galvana marked this conversation as resolved.
Show resolved Hide resolved
title: experience?.experience_config?.title || "",
onClose: dispatchCloseEvent,
});
Expand Down Expand Up @@ -153,6 +155,7 @@ const Overlay: FunctionComponent<Props> = ({
attributes={attributes}
experience={experience.experience_config}
onVendorPageClick={onVendorPageClick}
modalType={overlayType}
>
{renderModalContent({ onClose: handleCloseModal })}
</ConsentModal>
Expand Down
36 changes: 36 additions & 0 deletions clients/fides-js/src/components/PrivacyPolicyLink.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { h } from "preact";
import { ExperienceConfig } from "../lib/consent-types";

const PrivacyPolicyLink = ({
experience,
}: {
experience?: ExperienceConfig;
}) => {
if (
!experience?.privacy_policy_link_label ||
!experience?.privacy_policy_url
) {
return null;
}

return (
<div
style={{
display: "flex",
alignItems: "center",
justifyContent: "center",
}}
>
<a
href={experience.privacy_policy_url}
rel="noopener noreferrer"
target="_blank"
className="fides-privacy-policy"
>
{experience.privacy_policy_link_label}
</a>
</div>
);
};

export default PrivacyPolicyLink;
4 changes: 3 additions & 1 deletion clients/fides-js/src/components/Toggle.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ const Toggle = ({
disabled={disabled}
/>
{/* Mark as `hidden` so it will fall back to a regular checkbox if CSS is not available */}
<span className="fides-toggle-display" hidden />
<span className="fides-toggle-display" hidden>
{checked ? "Opt-in" : "Opt-out"}
</span>
</label>
);
};
Expand Down
Loading
Loading