-
Notifications
You must be signed in to change notification settings - Fork 355
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(Modal): promoted next version #10358
Conversation
Preview: https://patternfly-react-pr-10358.surge.sh A11y report: https://patternfly-react-pr-10358-a11y.surge.sh |
5dc20b5
to
b86c0d6
Compare
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.
General comment. We should not be using the deprecated modal in our components or examples/demos. We can open a follow up issue for that.
@@ -6,7 +6,7 @@ import { AboutModalBoxHeader } from './AboutModalBoxHeader'; | |||
import { AboutModalBoxBrand } from './AboutModalBoxBrand'; | |||
import { AboutModalBoxCloseButton } from './AboutModalBoxCloseButton'; | |||
import { AboutModalBox } from './AboutModalBox'; | |||
import { Modal, ModalVariant } from '../Modal'; |
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.
we should be using the new modal here. I am ok with follow up issue to do that, but it should be done for Beta.
@@ -16,6 +14,7 @@ import { | |||
Radio, | |||
Popper | |||
} from '@patternfly/react-core'; | |||
import { Modal as ModalDeprecated, ModalVariant as ModalVariantDeprecated } from '@patternfly/react-core/deprecated'; |
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.
same comment as above here
@@ -8,13 +9,13 @@ export const WizardWithinModal = () => { | |||
<> | |||
<Button onClick={() => setIsModalOpen(true)}>Show Modal</Button> | |||
|
|||
<Modal | |||
<ModalDeprecated |
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.
we should use new modal here.
In regards to above, followup created for updating examples: #10363 |
Your changes have been released in:
Thanks for your contribution! 🎉 |
What: Closes #8078
Additional issues: