docs(Alert): update AlertActionLink props so they follow guidelines#9666
Merged
tlabaj merged 4 commits intopatternfly:mainfrom Oct 16, 2023
Merged
docs(Alert): update AlertActionLink props so they follow guidelines#9666tlabaj merged 4 commits intopatternfly:mainfrom
tlabaj merged 4 commits intopatternfly:mainfrom
Conversation
Collaborator
|
Preview: https://patternfly-react-pr-9666.surge.sh A11y report: https://patternfly-react-pr-9666-a11y.surge.sh |
Contributor
thatblindgeye
left a comment
thatblindgeye
left a comment
There was a problem hiding this comment.
This is looking good, just a comment below.
Regarding your other comments:
- Personally I'd be in favor of replacing the
alertpopup with just a console log. It may be more important to keep these alerts on screen rather than allow them to be removed just for purposes of the examples. - Would make sense to combine or remove duplicate examples. I'm not sure if we really need 4 of the alerts in that particular "Variations" example to use a close button since we're mentioning a specific alert in the example description; maybe just keep that 4th Alert to show the actionClose prop? I'd delegate to @tlabaj and @andrew-ronaldson on this though.
- "Ignore" text acts as a button, not a link - alert variations examples are simpler - fixed a link to /components/alert/accessibility - AlertActionCloseButton will not trigger a popup alert
thatblindgeye
requested changes
Sep 27, 2023
Contributor
thatblindgeye
left a comment
thatblindgeye
left a comment
There was a problem hiding this comment.
One other comment below. Also can we update the alerts in the "Expandable alerts" example so that one has just the close button (no actionLinks), and the other has only actionLinks (no close button).
thatblindgeye
approved these changes
Oct 9, 2023
tlabaj
approved these changes
Oct 16, 2023
Collaborator
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What: Closes #9331
Additional issues:
Few things I'd also improve in Alert docs: