-
Notifications
You must be signed in to change notification settings - Fork 358
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
feat(ClipboardCopy): Convert clipboard copy to typescript #2131
Conversation
PatternFly-React preview: https://2131-pr-patternfly-react-patternfly.surge.sh |
Codecov Report
@@ Coverage Diff @@
## master #2131 +/- ##
=========================================
Coverage ? 80.54%
=========================================
Files ? 666
Lines ? 8470
Branches ? 729
=========================================
Hits ? 6822
Misses ? 1281
Partials ? 367
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! 🎖️
packages/patternfly-4/react-core/src/components/ClipboardCopy/ClipboardCopy.tsx
Outdated
Show resolved
Hide resolved
@@ -31,7 +31,7 @@ export interface TooltipProps extends Omit<HTMLProps<HTMLDivElement>, 'content' | |||
/** z-index of the tooltip */ | |||
zIndex?: number; | |||
/** Size of the tooltip */ | |||
maxWidth?: '12.5rem'; | |||
maxWidth?: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!
packages/patternfly-4/react-core/src/components/ClipboardCopy/ClipboardCopy.tsx
Outdated
Show resolved
Hide resolved
packages/patternfly-4/react-core/src/components/ClipboardCopy/ClipboardCopyButton.tsx
Outdated
Show resolved
Hide resolved
packages/patternfly-4/react-core/src/components/ClipboardCopy/ClipboardCopyExpanded.tsx
Outdated
Show resolved
Hide resolved
packages/patternfly-4/react-core/src/components/ClipboardCopy/ClipboardCopyToggle.tsx
Outdated
Show resolved
Hide resolved
/** Flag to show if the input is read only. */ | ||
isReadOnly?: boolean; | ||
/** Adds Clipboard Copy variant styles. */ | ||
variant?: typeof ClipboardCopyVariant | 'inline' | 'expansion'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we decided to just have the union of the values here and not the "Typeof"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that we need the typeof for the older consumers using the enum.
packages/patternfly-4/react-core/src/components/ClipboardCopy/ClipboardCopy.tsx
Outdated
Show resolved
Hide resolved
packages/patternfly-4/react-core/src/components/ClipboardCopy/ClipboardCopyButton.tsx
Outdated
Show resolved
Hide resolved
it('Verify content expands', () => { | ||
cy.get('.pf-c-clipboard-copy__group-toggle').click(); | ||
cy.get('.pf-c-clipboard-copy').should('have.class', 'pf-m-expanded'); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should add more verification steps here. We dod not verify that the copy actually works and I think we need to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know of a good way to test the copy paste feature? I tried this:
it('Verify copy to clipboard works', () => {
const form = cy.get('.pf-c-form-control');
form.clear().type('Hi');
cy.get('.pf-c-clipboard-copy__group-copy').click();
cy.get('.pf-c-form-control').clear().type('{cmd}v');
form.should('have.value', 'Hi');
});
But it always ends up typing "v" instead of doing a paste. I've been looking into it, but I'm not sure what the solution would be: cypress-io/cypress#3316
6ff9c33
to
762c9fe
Compare
PatternFly-React preview: https://patternfly-react-pr-2131.surge.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Fixes #2130