Added warnings to the `PageContentWide` component #3127

chloe0592 · 2023-07-04T11:00:11Z

The PageContentWide doesn't throw a warning when more children are passed as props.
For example if 3 are passed, it only renders first 2 children and it goes unnoticed that 3rd component is not rendered and why is that the case.

In this PR we cover 2 possible scenarios when the warning is shown:

There's 1 column and more than 1 child.
There's 1/1 or 2/1 columns and more than 2 children.

This PR closes - #3126.

changeset-bot · 2023-07-04T11:00:15Z

🦋 Changeset detected

Latest commit: c7557ec

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 35 packages

Name	Type
@commercetools-frontend/application-components	Patch
@commercetools-local/visual-testing-app	Patch
@commercetools-frontend/application-shell	Patch
@commercetools-frontend/codemod	Patch
@commercetools-frontend/mc-scripts	Patch
@commercetools-applications/merchant-center-template-starter-typescript	Patch
@commercetools-applications/merchant-center-template-starter	Patch
@commercetools-local/playground	Patch
@commercetools-website/components-playground	Patch
@commercetools-frontend/cypress	Patch
@commercetools-backend/eslint-config-node	Patch
@commercetools-backend/express	Patch
@commercetools-backend/loggers	Patch
@commercetools-frontend/actions-global	Patch
@commercetools-frontend/application-config	Patch
@commercetools-frontend/application-shell-connectors	Patch
@commercetools-frontend/assets	Patch
@commercetools-frontend/babel-preset-mc-app	Patch
@commercetools-frontend/browser-history	Patch
@commercetools-frontend/constants	Patch
@commercetools-frontend/create-mc-app	Patch
@commercetools-frontend/eslint-config-mc-app	Patch
@commercetools-frontend/i18n	Patch
@commercetools-frontend/jest-preset-mc-app	Patch
@commercetools-frontend/jest-stylelint-runner	Patch
@commercetools-frontend/l10n	Patch
@commercetools-frontend/mc-dev-authentication	Patch
@commercetools-frontend/mc-html-template	Patch
@commercetools-frontend/notifications	Patch
@commercetools-frontend/permissions	Patch
@commercetools-frontend/react-notifications	Patch
@commercetools-frontend/sdk	Patch
@commercetools-frontend/sentry	Patch
@commercetools-frontend/url-utils	Patch
@commercetools-website/custom-applications	Patch

Not sure what this means? Click here to learn what changesets are.

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

...on-components/src/components/page-content-containers/page-content-wide/page-content-wide.tsx

github-actions · 2023-07-04T11:04:13Z

Deploy preview for merchant-center-application-kit ready!

✅ Preview
https://merchant-center-application-bwq72oclh-commercetools.vercel.app
https://appkit-sha-a4f18e806e8b54b9af81a0f4faf9e7e9d71d3631.commercetools.vercel.app
https://appkit-pr-3127.commercetools.vercel.app

Built with commit c7557ec.
This pull request is being automatically deployed with vercel-action

jaikumar-tj

Thanks for the quick fixup. Just curious, does useWarning simply show a console warning or does it throw an error and prevent the developer from making mistake?

chloe0592 · 2023-07-04T12:02:32Z

Thanks for the quick fixup. Just curious, does useWarning simply show a console warning or does it throw an error and prevent the developer from making mistake?

@jaikumar-tj yes, exactly it'll show a warning in the console, it doesn’t throw an error. It will be looking like this.

CarlosCortizasCT · 2023-07-04T14:04:29Z

Thanks for the quick fixup. Just curious, does useWarning simply show a console warning or does it throw an error and prevent the developer from making mistake?

@jaikumar-tj yes, exactly it'll show a warning in the console, it doesn’t throw an error. It will be looking like this.

@jaikumar-tj we decided to follow @tdeekens advice of using a warning.

Bear in mind this will use a console.warn which should make the MC frontend consuming component tests to fail.

...on-components/src/components/page-content-containers/page-content-wide/page-content-wide.tsx

.changeset/great-eggs-try.md

ddouglasz

Nice! 🫶🏻
Just tiny observations

ddouglasz · 2023-07-05T08:04:40Z

...on-components/src/components/page-content-containers/page-content-wide/page-content-wide.tsx


  useWarning(
-    props.columns !== '1' || !Boolean(rightChild),
+    props.columns === '1' && childrenCount > 1,
    'PageContentWide: This component only renders its first children when using a single column but you provided more that one.'


Suggested change

'PageContentWide: This component only renders its first children when using a single column but you provided more that one.'

'PageContentWide: This component only renders its first children when using a single column but you provided more than one.'

ddouglasz · 2023-07-05T08:04:56Z

...on-components/src/components/page-content-containers/page-content-wide/page-content-wide.tsx

    'PageContentWide: This component only renders its first children when using a single column but you provided more that one.'
  );

+  useWarning(
+    props.columns !== '1' && childrenCount > 2,
+    'PageContentWide: This component only renders its first two children when using a two columns layout but you provided more that two.'


Suggested change

'PageContentWide: This component only renders its first two children when using a two columns layout but you provided more that two.'

'PageContentWide: This component only renders its first two children when using a two columns layout but you provided more than two.'

Great catch! 🦅 Thanks - c7557ec.

CarlosCortizasCT

💯

chore: added warnings to the page-conent-wide component

3cad5be

chloe0592 requested review from jaikumar-tj and a team July 4, 2023 11:00

chloe0592 self-assigned this Jul 4, 2023

chloe0592 commented Jul 4, 2023

View reviewed changes

...on-components/src/components/page-content-containers/page-content-wide/page-content-wide.tsx Outdated Show resolved Hide resolved

github-actions bot deployed to Preview July 4, 2023 11:03 View deployment

chore: updating test

f4ef151

github-actions bot deployed to Preview July 4, 2023 11:37 View deployment

jaikumar-tj approved these changes Jul 4, 2023

View reviewed changes

tdeekens approved these changes Jul 4, 2023

View reviewed changes

chore: updating the warnings

aaa1609

github-actions bot deployed to Preview July 5, 2023 07:39 View deployment

CarlosCortizasCT approved these changes Jul 5, 2023

View reviewed changes

...on-components/src/components/page-content-containers/page-content-wide/page-content-wide.tsx Show resolved Hide resolved

.changeset/great-eggs-try.md Outdated Show resolved Hide resolved

ddouglasz approved these changes Jul 5, 2023

View reviewed changes

chore: updated warnings and changelog

c7557ec

github-actions bot deployed to Preview July 5, 2023 08:18 View deployment

CarlosCortizasCT approved these changes Jul 5, 2023

View reviewed changes

chloe0592 merged commit 0918b46 into main Jul 5, 2023

chloe0592 deleted the kl-pageContentWide-warning branch July 5, 2023 08:36

merchant-center-automation mentioned this pull request Jul 5, 2023

Version Packages #3124

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added warnings to the `PageContentWide` component #3127

Added warnings to the `PageContentWide` component #3127

chloe0592 commented Jul 4, 2023 •

edited

Loading

changeset-bot bot commented Jul 4, 2023 •

edited

Loading

github-actions bot commented Jul 4, 2023 •

edited

Loading

jaikumar-tj left a comment

chloe0592 commented Jul 4, 2023

CarlosCortizasCT commented Jul 4, 2023

ddouglasz left a comment

ddouglasz Jul 5, 2023

ddouglasz Jul 5, 2023

chloe0592 Jul 5, 2023

CarlosCortizasCT left a comment

	'PageContentWide: This component only renders its first children when using a single column but you provided more that one.'
	'PageContentWide: This component only renders its first children when using a single column but you provided more than one.'

Added warnings to the PageContentWide component #3127

Added warnings to the PageContentWide component #3127

Conversation

chloe0592 commented Jul 4, 2023 • edited Loading

changeset-bot bot commented Jul 4, 2023 • edited Loading

🦋 Changeset detected

github-actions bot commented Jul 4, 2023 • edited Loading

jaikumar-tj left a comment

Choose a reason for hiding this comment

chloe0592 commented Jul 4, 2023

CarlosCortizasCT commented Jul 4, 2023

ddouglasz left a comment

Choose a reason for hiding this comment

ddouglasz Jul 5, 2023

Choose a reason for hiding this comment

ddouglasz Jul 5, 2023

Choose a reason for hiding this comment

chloe0592 Jul 5, 2023

Choose a reason for hiding this comment

CarlosCortizasCT left a comment

Choose a reason for hiding this comment

Added warnings to the `PageContentWide` component #3127

Added warnings to the `PageContentWide` component #3127

chloe0592 commented Jul 4, 2023 •

edited

Loading

changeset-bot bot commented Jul 4, 2023 •

edited

Loading

github-actions bot commented Jul 4, 2023 •

edited

Loading