Fix NotAuthorized default actions and clean the code#100
Fix NotAuthorized default actions and clean the code#100jessiehuff merged 10 commits intopatternfly:mainfrom
Conversation
fhlavac
commented
Jan 31, 2024
- fixed NotAuthorized default primary actions variant to primary
- cleaned the CloseButton props
- Updated docs to contain reference to the parent components props
nicolethoen
left a comment
There was a problem hiding this comment.
couple questions about the style overrides in the CloseButton. They could be turned into follow up issues if need be.
| import { createUseStyles } from 'react-jss'; | ||
| import clsx from 'clsx'; | ||
|
|
||
| const useStyles = createUseStyles({ |
There was a problem hiding this comment.
@jessiehuff does removing the padding on this component keep the click area large enough to meet a11y standards?
And I think that in general, to keep these components compatible with right-to-left languages, we might want to avoid using float:right. I think there is a float:end or something like that we should be using. I'm also curious though if there's a PF modifier we could use rather than this custom styling. @mcoker WDYT?
There was a problem hiding this comment.
@nicolethoen good call on the RTL support. There are logical values for float - "inline-end" (used instead of "right" - what you'd use here) and "inline-start" (used instead of "left").
I'm also curious though if there's a PF modifier we could use rather than this custom styling
We don't have a global modifier for this. We do have float utility classes though, and that utility stylesheet is lightweight (https://unpkg.com/browse/@patternfly/patternfly@latest/utilities/Float/float.css) if that's an option, though it's worth noting that our utility classes are not using logical values when they probably should. I opened an issue for that here patternfly/patternfly#6273.
There was a problem hiding this comment.
Based on WCAG guidelines, the clickable area should be 44 x 44 pixels. It looks like removing the padding makes it 48 x 36 so we should probably at least increase the height to 44 pixels.
There was a problem hiding this comment.
i think we are going to need a designer to step in at some point to commit a review once or twice a months of new components. with the changes in v6 I can see this giving us headaches. Fortunately we are small enough right now w can fix stuff.
.../patternfly-docs/content/extensions/component-groups/examples/NotAuthorized/NotAuthorized.md
Outdated
Show resolved
Hide resolved
...le/patternfly-docs/content/extensions/component-groups/examples/WarningModal/WarningModal.md
Outdated
Show resolved
Hide resolved
.../patternfly-docs/content/extensions/component-groups/examples/NotAuthorized/NotAuthorized.md
Outdated
Show resolved
Hide resolved
|
A couple of additional phrasing changes (I've already made the changes in the text below): in errorstate.md
in warningmodal.md (I can't preview the interactive example, so I may be wrong about the functionality - lmk if so or if I could better expand)
|
…roups/examples/NotAuthorized/NotAuthorized.md Co-authored-by: Erin Donehoo <105813956+edonehoo@users.noreply.github.com>
…roups/examples/WarningModal/WarningModal.md Co-authored-by: Erin Donehoo <105813956+edonehoo@users.noreply.github.com>
…roups/examples/NotAuthorized/NotAuthorized.md Co-authored-by: Erin Donehoo <105813956+edonehoo@users.noreply.github.com>
|
@nicolethoen @edonehoo thank you, the comments should be addressed. I've also changed type of the |
...le/patternfly-docs/content/extensions/component-groups/examples/WarningModal/WarningModal.md
Show resolved
Hide resolved
|
@edonehoo updated, thank you 🙂 |
|
🎉 This PR is included in version 5.1.0-prerelease.5 🎉 The release is available on: Your semantic-release bot 📦🚀 |