Skip to content

Remove email campaigns factory#148

Draft
raphaelblum wants to merge 5 commits intonextfrom
remove-email-campaigns-factory
Draft

Remove email campaigns factory#148
raphaelblum wants to merge 5 commits intonextfrom
remove-email-campaigns-factory

Conversation

@raphaelblum
Copy link
Collaborator

@raphaelblum raphaelblum commented Nov 15, 2024

depends on !147

COM-1053

Description

The factory is not needed. I'd like to simplify creating the EmailCampaignsPage.

@raphaelblum raphaelblum force-pushed the remove-email-campaigns-factory branch from 29758fb to dd52872 Compare November 15, 2024 12:27
primary: <FormattedMessage id="menu.newsletter.emailCampaigns" defaultMessage="Email campaigns" />,
route: {
path: "/newsletter/email-campaigns",
component: () => <EmailCampaignsPage EmailCampaignContentBlock={EmailCampaignContentBlock} />,
Copy link
Member

@thomasdax98 thomasdax98 Nov 20, 2024

Choose a reason for hiding this comment

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

Suggested change
component: () => <EmailCampaignsPage EmailCampaignContentBlock={EmailCampaignContentBlock} />,
render: () => <EmailCampaignsPage EmailCampaignContentBlock={EmailCampaignContentBlock} />,

This must be render or the component will unmount on every render: https://v5.reactrouter.com/web/api/Route/component#:~:text=That%20means%20if%20you%20provide%20an%20inline%20function%20to%20the%20component%20prop%2C%20you%20would%20create%20a%20new%20component%20every%20render.

"@comet/brevo-admin": major
---

Remove factory createEmailCampaignsPage
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Remove factory createEmailCampaignsPage
Remove `createEmailCampaignsPage` factory

@RainbowBunchie
Copy link
Contributor

This needs a rebase @raphaelblum 🙏🏻

@raphaelblum raphaelblum marked this pull request as draft March 28, 2025 08:19
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