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.

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

GEN-637 - feat: added TrustpilotBlock #2711

Merged
merged 1 commit into from
Jul 10, 2023

Conversation

guilhermespopolin
Copy link
Contributor

@guilhermespopolin guilhermespopolin commented Jul 7, 2023

Describe your changes

  • Added a TrustpilotBlock that displays Trustpilot data.

How the whole thing works:

  1. There's a new service - apps/store/src/services/trustpilot/trustpilot.tsx - which fetch that data from Trustpilot API and store it in a Jotai's atom so it can be available through the whole app. We then fetch that data and proper hydrate the atom in the apps/store/src/pages/[[...slug]].tsx and apps/store/src/components/LayoutWithMenu/LayoutWithMenu.tsx files respectively.
  2. TrustpilotBlock then reads that data from the atom and proper display it.
Screenshot 2023-07-06 at 17 41 54

Justify why they are needed

Requested by design/editors

Can be tested here

@vercel
Copy link

vercel bot commented Jul 7, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
hedvig-dot-com ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 10, 2023 0:52am
onboarding ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 10, 2023 0:52am

@guilhermespopolin
Copy link
Contributor Author

@guilhermespopolin guilhermespopolin changed the title feat: added TrustpilotBlock GEN-637 - feat: added TrustpilotBlock Jul 7, 2023
@notion-workspace
Copy link

Copy link
Member

@gustaveen gustaveen left a comment

Choose a reason for hiding this comment

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

Nice work! 🙌
On thing to consider is to put the text and url as fields in Storyblok instead. Then we could make a reusable block of it

apps/store/src/blocks/TrustpilotBlock.tsx Outdated Show resolved Hide resolved
apps/store/src/blocks/TrustpilotBlock.tsx Outdated Show resolved Hide resolved
@guilhermespopolin
Copy link
Contributor Author

Nice work! 🙌 On thing to consider is to put the text and url as fields in Storyblok instead. Then we could make a reusable block of it

Have update it so we get the link from Storyblok

Copy link
Member

@gustaveen gustaveen left a comment

Choose a reason for hiding this comment

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

Awesome! 🙌

Copy link
Contributor

@robinandeer robinandeer left a comment

Choose a reason for hiding this comment

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

Look good!

Comment on lines 46 to 47
paddingBlock: '10rem',
paddingInline: '1rem',
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use theme?


const ScoreText = styled(Text)({
lineHeight: '1.32',
// Font size tihs high is an edge case. That's why I'm not using Text's 'size' prop
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: typo "tihs"

@guilhermespopolin
Copy link
Contributor Author

@guilhermespopolin started a stack merge that includes this pull request via Graphite.

@guilhermespopolin guilhermespopolin force-pushed the GEN-637/feat/trustpilot-block_split_1 branch from 91abec3 to 0e6b172 Compare July 10, 2023 12:41
Base automatically changed from GEN-637/feat/trustpilot-block_split_1 to main July 10, 2023 12:42
@guilhermespopolin
Copy link
Contributor Author

Graphite rebased this pull request as part of a merge.

@guilhermespopolin guilhermespopolin force-pushed the GEN-637/feat/trustpilot-block_split_2 branch from e2bf84a to 8b554d4 Compare July 10, 2023 12:42
@guilhermespopolin guilhermespopolin merged commit 53e94df into main Jul 10, 2023
3 checks passed
@guilhermespopolin guilhermespopolin deleted the GEN-637/feat/trustpilot-block_split_2 branch July 10, 2023 12:43
@guilhermespopolin
Copy link
Contributor Author

@guilhermespopolin merged this pull request with Graphite.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants