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

Jump to bottom

Add cookie banner and cookie policy page #1335

Merged
merged 2 commits into from
Oct 18, 2024
Merged

Add cookie banner and cookie policy page #1335

merged 2 commits into from
Oct 18, 2024

Conversation

roshni73
Copy link
Collaborator

@roshni73 roshni73 commented Aug 28, 2024 •
edited by samshara
Loading

Addresses:

Changes

  • Add Banner and cookie policy page

This PR doesn't introduce:

  • typos
  • conflict markers
  • unwanted comments
  • temporary files, auto-generated files or secret keys
  • console.log meant for debugging
  • codegen errors

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Aug 28, 2024 •
edited
Loading

⚠️ No Changeset found

Latest commit: bf55ccc

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@roshni73 roshni73 marked this pull request as draft August 28, 2024 11:32
@roshni73 roshni73 marked this pull request as ready for review August 29, 2024 10:23
shreeyash07
shreeyash07 approved these changes Aug 30, 2024
View reviewed changes
app/src/views/RootLayout/index.tsx Outdated
Comment on lines 67 to 79
const handleAcceptClick = () => {
setIsCookiesBannerVisible(false);
};

const handleLinkClick = () => {
setIsCookiesBannerVisible(false);
};

Copy link
Collaborator

Choose a reason for hiding this comment

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

This 2 function are doing same thing.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Make it one and use useCallback function

@shreeyash07 shreeyash07 self-requested a review August 30, 2024 06:19
barshathakuri
barshathakuri requested changes Aug 30, 2024
View reviewed changes
app/src/views/CookiePolicy/index.tsx Outdated
{strings.useOfOurInformationDescriptionLink}
</Link>
</div>

Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove extra line.

app/src/views/CookiePolicy/index.tsx
publishedDay: 'November',
publishedDate: 29,
publishedYear: 2021,
})}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above

Copy link
Member

Choose a reason for hiding this comment

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

@barshathakuri While leaving of comments and reviews, let's not use "Same as above" as the context might get lost because of multiple reasons.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Noted!

app/src/views/CookiePolicy/index.tsx Outdated
)}
/>
<Container
heading={strings.AutomaticallyCollectedHeading}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
heading={strings.AutomaticallyCollectedHeading}
heading={strings.automaticallyCollectedHeading}

app/src/views/CookiePolicy/index.tsx Outdated
footerContentClassName={styles.headerDescription}
footerContent={(
<>
<div>{strings.AutomaticallyCollectedDescription}</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<div>{strings.AutomaticallyCollectedDescription}</div>
<div>{strings.automaticallyCollectedDescription}</div>

app/src/views/CookiePolicy/index.tsx Outdated
Comment on lines 158 to 156
<ul>
<li>{strings.AutomaticallyCollectedList1}</li>
<li>{strings.AutomaticallyCollectedList2}</li>
<li>{strings.AutomaticallyCollectedList3}</li>
<li>{strings.AutomaticallyCollectedList4}</li>
<li>{strings.AutomaticallyCollectedList5}</li>
</ul>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above.

app/src/views/CookiePolicy/index.tsx Outdated
{
termsLink: (
<Link
href="[email protected]"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above

app/src/views/CookiePolicy/index.tsx Outdated
</strong>
</div>
</>

Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove extra line

app/src/views/CookiePolicy/styles.module.css Outdated
Comment on lines 1 to 4
.cookie-page{
display: flex;

.side-titles{
display: flex;
flex-direction: column;
padding: var(--go-ui-spacing-sm);
gap: var(--go-ui-spacing-md);
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
.cookie-page{
display: flex;
.side-titles{
display: flex;
flex-direction: column;
padding: var(--go-ui-spacing-sm);
gap: var(--go-ui-spacing-md);
}
.cookie-page {
display: flex;
.side-titles {
display: flex;
flex-direction: column;
padding: var(--go-ui-spacing-sm);
gap: var(--go-ui-spacing-md);
}

app/src/views/RootLayout/index.tsx Outdated
Comment on lines 399 to 411
headingDescription={strings.cookiesBannerDescription}
icons={<AlertInformationLineIcon className={styles.alertInfoIcon} />}
footerActions={(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
headingDescription={strings.cookiesBannerDescription}
icons={<AlertInformationLineIcon className={styles.alertInfoIcon} />}
footerActions={(
headingDescription={strings.cookiesBannerDescription}
icons={
<AlertInformationLineIcon
className={styles.alertInfoIcon}
/>
}
footerActions={(

app/src/views/RootLayout/styles.module.css Outdated
Comment on lines 21 to 33
.cookies-banner{
position: sticky;
bottom: 0;
background-color: var(--go-ui-color-primary-blue);
padding: var(--go-ui-spacing-xl);
color: var(--go-ui-color-white);
font-weight: var(--go-ui-font-weight-medium);

.alert-info-icon{
font-size: 3rem;

}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
.cookies-banner{
position: sticky;
bottom: 0;
background-color: var(--go-ui-color-primary-blue);
padding: var(--go-ui-spacing-xl);
color: var(--go-ui-color-white);
font-weight: var(--go-ui-font-weight-medium);
.alert-info-icon{
font-size: 3rem;
}
}
.cookies-banner {
position: sticky;
bottom: 0;
background-color: var(--go-ui-color-primary-blue);
padding: var(--go-ui-spacing-xl);
color: var(--go-ui-color-white);
font-weight: var(--go-ui-font-weight-medium);
.alert-info-icon{
font-size: 3rem;
}
}

@samshara samshara changed the title Add Banner and cookie policy page Add cookie banner and cookie policy page Sep 12, 2024
@tnagorra tnagorra assigned frozenhelium and unassigned samshara Oct 8, 2024
@AdityaKhatri AdityaKhatri dismissed stale reviews from tnagorra and barshathakuri October 18, 2024 05:41

Because me

@AdityaKhatri AdityaKhatri merged commit e084f35 into develop Oct 18, 2024
9 checks passed
@AdityaKhatri AdityaKhatri deleted the feature/cookies-page-banner branch October 18, 2024 05:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AdityaKhatri AdityaKhatri AdityaKhatri approved these changes

@samshara samshara Awaiting requested review from samshara

@puranban puranban Awaiting requested review from puranban

@shreeyash07 shreeyash07 Awaiting requested review from shreeyash07

@barshathakuri barshathakuri Awaiting requested review from barshathakuri

@tnagorra tnagorra Awaiting requested review from tnagorra

Assignees

@frozenhelium frozenhelium

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@roshni73 @tnagorra @AdityaKhatri @shreeyash07 @barshathakuri @frozenhelium @samshara @subinasr