fix(modal): add support for status modifiers on title#6180
fix(modal): add support for status modifiers on title#6180mcoker merged 2 commits intopatternfly:mainfrom
Conversation
|
Preview: https://patternfly-pr-6180.surge.sh A11y report: https://patternfly-pr-6180-a11y.surge.sh |
srambach
left a comment
srambach
left a comment
There was a problem hiding this comment.
Looks good - is it worth saying that if the modifier is placed on the title, it will override the modifier on the modal box? I know the react component won't do this, but if someone does it, it might be confusing. Or not, I'm fine if you don't think it's necessary.
Will we phase out allowing the modifier on the modal box?
@srambach probably not, just from the wording suggesting it goes on one or the other (and not both)? But I could go either way, though I may need some help wording that properly 😅
Yeah, assuming this update works out for the next modal, it would be great to just go with a single approach. There is still a possibility adding the modifier to the title like this is too narrowly scoped (like we also want to draw a status color border around the modal or something like that), but I think we have some time to test that out before the next component is promoted and the current one is deprecated. I went back and added a beta tag to the new example so we can change it if we need to. WDYT - good to merge or want to discuss any more updates? |
|
🎉 This PR is included in version 5.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 5.3.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
4 participants
fixes #6091