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

feat: react and share (Askem) component #361

Merged
merged 12 commits into from
Jun 22, 2023
Merged

feat: react and share (Askem) component #361

merged 12 commits into from
Jun 22, 2023

Conversation

melniiv
Copy link
Contributor

@melniiv melniiv commented Jun 14, 2023 •
edited

Description

  • Implements the Askem (react and share component)
  • Implements cookie consent page in each monorepo app
  • Cookie consent modal is now disabled for the cookie consent page
  • When accepting the consent, the matomo and feedback modules are refreshed with updated configs (if accepted from the cookie consent page, and redirectPath defined, user gets redirected to the initial page right after acceptation)
  • Translations in cookie consent fixed
  • Feedback is a part of footer component, as it always has to appear above the footer (if not disabled. for example we don't have feedback in front page and cookie consent page). First i wanted to add new prop to the HCRC page content, but for different pages implementation varies, so it would be much more changes to the code, including HCRC. So footer is the only suitable place for feedback.

Issues

Closes

DEV-XXX:

Related

Testing

Automated tests

Manual testing

Screenshots

Additional notes

@terovirtanen
Copy link
Contributor

VENUE GRAPHQL PROXY is deployed to platta: https://venue-graphql-proxy-pr361.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

EVENTS GRAPHQL PROXY is deployed to platta: https://events-graphql-proxy-pr361.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

Hobbies-Helsinki branch is deployed to platta: https://harrastukset-pr361.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

Events-Helsinki branch is deployed to platta: https://tapahtumat-pr361.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

TestCafe result is success for https://harrastukset-pr361.dev.hel.ninja 😆🎉🎉🎉

@terovirtanen
Copy link
Contributor

Sports-Helsinki branch is deployed to platta: https://liikunta-pr361.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

TestCafe result is success for https://tapahtumat-pr361.dev.hel.ninja 😆🎉🎉🎉

@terovirtanen
Copy link
Contributor

VENUE GRAPHQL PROXY is deployed to platta: https://venue-graphql-proxy-pr361.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

EVENTS GRAPHQL PROXY is deployed to platta: https://events-graphql-proxy-pr361.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

TestCafe result is success for https://liikunta-pr361.dev.hel.ninja 😆🎉🎉🎉

@terovirtanen
Copy link
Contributor

Hobbies-Helsinki branch is deployed to platta: https://harrastukset-pr361.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

Events-Helsinki branch is deployed to platta: https://tapahtumat-pr361.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

Sports-Helsinki branch is deployed to platta: https://liikunta-pr361.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

TestCafe result is success for https://harrastukset-pr361.dev.hel.ninja 😆🎉🎉🎉

@terovirtanen
Copy link
Contributor

VENUE GRAPHQL PROXY is deployed to platta: https://venue-graphql-proxy-pr361.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

TestCafe result is failed for https://tapahtumat-pr361.dev.hel.ninja 😿💢💥💥

@terovirtanen
Copy link
Contributor

TestCafe result is success for https://liikunta-pr361.dev.hel.ninja 😆🎉🎉🎉

@terovirtanen
Copy link
Contributor

EVENTS GRAPHQL PROXY is deployed to platta: https://events-graphql-proxy-pr361.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

Hobbies-Helsinki branch is deployed to platta: https://harrastukset-pr361.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

Events-Helsinki branch is deployed to platta: https://tapahtumat-pr361.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

TestCafe result is failed for https://harrastukset-pr361.dev.hel.ninja 😿💢💥💥

@terovirtanen
Copy link
Contributor

Sports-Helsinki branch is deployed to platta: https://liikunta-pr361.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

EVENTS GRAPHQL PROXY is deployed to platta: https://events-graphql-proxy-pr361.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

VENUE GRAPHQL PROXY is deployed to platta: https://venue-graphql-proxy-pr361.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

TestCafe result is failed for https://tapahtumat-pr361.dev.hel.ninja 😿💢💥💥

@terovirtanen
Copy link
Contributor

TestCafe result is success for https://liikunta-pr361.dev.hel.ninja 😆🎉🎉🎉

@terovirtanen
Copy link
Contributor

Sports-Helsinki branch is deployed to platta: https://liikunta-pr361.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

TestCafe result is failed for https://harrastukset-pr361.dev.hel.ninja 😿💢💥💥

@terovirtanen
Copy link
Contributor

EVENTS GRAPHQL PROXY is deployed to platta: https://events-graphql-proxy-pr361.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

TestCafe result is success for https://liikunta-pr361.dev.hel.ninja 😆🎉🎉🎉

@terovirtanen
Copy link
Contributor

VENUE GRAPHQL PROXY is deployed to platta: https://venue-graphql-proxy-pr361.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

TestCafe result is failed for https://tapahtumat-pr361.dev.hel.ninja 😿💢💥💥

nikomakela
nikomakela approved these changes Jun 22, 2023
View reviewed changes
Copy link
Contributor

@nikomakela nikomakela left a comment

Choose a reason for hiding this comment

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

Couple of trivial improvements suggested, but all good

packages/components/src/app/BaseApp.tsx Outdated
const askemFeedbackInstance = React.useMemo(
() => createAskemInstance(askemFeedbackConfiguration),
// eslint-disable-next-line react-hooks/exhaustive-deps
[createAskemInstance]
Copy link
Contributor

Choose a reason for hiding this comment

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

Trivial, but I would say that the askemFeedbackConfiguration is more important here than the createAskemInstance. Instead of disabling the exhaustive-deps check, I would just add both variables to the deps.

packages/components/src/components/askem/Askem.ts Outdated Show resolved Hide resolved
packages/components/src/components/askem/index.ts Outdated
export { default as MatomoProvider } from './AskemProvider';
export { default as createAskemInstance } from './instance';
export { default as AskemFeedbackContainer } from './AskemFeedbackContainer';
// export { default as useMatomo } from './useAskem';
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be removed, because it is confusing

@sonarcloud
Copy link

sonarcloud bot commented Jun 22, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@sonarcloud
Copy link

sonarcloud bot commented Jun 22, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@sonarcloud
Copy link

sonarcloud bot commented Jun 22, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

0.0% 0.0% Coverage
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@sonarcloud
Copy link

sonarcloud bot commented Jun 22, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

0.0% 0.0% Coverage
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@sonarcloud
Copy link

sonarcloud bot commented Jun 22, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

0.0% 0.0% Coverage
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@terovirtanen
Copy link
Contributor

Hobbies-Helsinki branch is deployed to platta: https://harrastukset-pr361.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

Events-Helsinki branch is deployed to platta: https://tapahtumat-pr361.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

Sports-Helsinki branch is deployed to platta: https://liikunta-pr361.dev.hel.ninja 🚀🚀🚀

@terovirtanen
Copy link
Contributor

TestCafe result is failed for https://harrastukset-pr361.dev.hel.ninja 😿💢💥💥

@terovirtanen
Copy link
Contributor

TestCafe result is failed for https://tapahtumat-pr361.dev.hel.ninja 😿💢💥💥

@terovirtanen
Copy link
Contributor

TestCafe result is success for https://liikunta-pr361.dev.hel.ninja 😆🎉🎉🎉

@terovirtanen
Copy link
Contributor

VENUE GRAPHQL PROXY is deployed to platta: https://venue-graphql-proxy-pr361.dev.hel.ninja 🚀🚀🚀

@melniiv melniiv merged commit ed076be into main Jun 22, 2023
98 of 106 checks passed
This was referenced Jun 22, 2023
Merged
Merged
Merged
This was referenced Jan 5, 2024
Merged
Merged
This was referenced Apr 11, 2024
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nikomakela nikomakela nikomakela approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@melniiv @terovirtanen @nikomakela