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

fix(ClipboardCopy): Add string[] type to children #11177

Merged
merged 2 commits into from
Jan 24, 2025

Conversation

Venefilyn
Copy link
Contributor

@Venefilyn Venefilyn commented Nov 13, 2024

The ClipboardCopy component already handles string[] but didn't have it written in the props as valid.

What: Closes #11186

Additional issues:

@patternfly-build
Copy link
Contributor

patternfly-build commented Nov 13, 2024

@Venefilyn Venefilyn force-pushed the fix/string-array-clipboard-copy branch from ce0fdf3 to f1671dd Compare November 18, 2024 15:18
@Venefilyn
Copy link
Contributor Author

Actually it doesn't handle it correctly for some reason. @tlabaj do you happen to know what's happening?

image

@Venefilyn Venefilyn force-pushed the fix/string-array-clipboard-copy branch from f1671dd to 1325248 Compare November 18, 2024 15:19
@Venefilyn
Copy link
Contributor Author

Ah nvm found the issue, was inside componentDidUpdate() 🙏

@thatblindgeye thatblindgeye requested review from a team, tlabaj and thatblindgeye and removed request for a team January 13, 2025 13:45
Copy link
Contributor

@thatblindgeye thatblindgeye left a comment

Choose a reason for hiding this comment

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

This looks good, just a linting error due to formatting that needs to be resolved.

Copy link
Contributor

@tlabaj tlabaj left a comment

Choose a reason for hiding this comment

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

Can you please add a unit test for the array of strings type?

@Venefilyn Venefilyn force-pushed the fix/string-array-clipboard-copy branch 4 times, most recently from 3ec6591 to 8e6dd02 Compare January 21, 2025 14:05
The ClipboardCopy component already handles string[] but didn't have it
written in the props as valid.
@Venefilyn Venefilyn force-pushed the fix/string-array-clipboard-copy branch from 8e6dd02 to cb3204f Compare January 21, 2025 14:18
@Venefilyn
Copy link
Contributor Author

@tlabaj I've updated and fixed tests. It does fail atm but I do not understand why as the Jest docs claims it should work https://jestjs.io/docs/expect#tohavebeencalledwitharg1-arg2-

I have trouble getting jest to work locally unfortunately as it keeps giving me this error, so debugging is a bit time consuming for me. Could you fix it instead or let me know if I missed a step in preparing my dev environment?

FAIL  packages/react-core/src/deprecated/components/Modal/__tests__/ModalBoxDescription.test.tsx
   Test suite failed to run

    Cannot find module '@patternfly/react-styles' from 'packages/react-core/src/deprecated/components/Modal/ModalBoxDescription.tsx'

    Require stack:
      packages/react-core/src/deprecated/components/Modal/ModalBoxDescription.tsx
      packages/react-core/src/deprecated/components/Modal/__tests__/ModalBoxDescription.test.tsx

      1 | import * as React from 'react';
    > 2 | import { css } from '@patternfly/react-styles';

@dlabaj dlabaj merged commit 33feb10 into patternfly:main Jan 24, 2025
13 checks passed
@patternfly-build
Copy link
Contributor

Your changes have been released in:

  • @patternfly/react-code-editor@6.2.0-prerelease.11
  • @patternfly/react-core@6.2.0-prerelease.10
  • @patternfly/react-docs@7.2.0-prerelease.20
  • @patternfly/react-drag-drop@6.2.0-prerelease.11
  • demo-app-ts@6.0.0-prerelease.59
  • @patternfly/react-table@6.2.0-prerelease.11
  • @patternfly/react-templates@6.2.0-prerelease.11

Thanks for your contribution! 🎉

@Venefilyn Venefilyn deleted the fix/string-array-clipboard-copy branch January 24, 2025 22:34
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.

Bug - ClipboardCopy - TS type does not allow string[] children
5 participants