Skip to content

feat(Login): use children for loginmainfooterlinksitem#10107

Merged
tlabaj merged 2 commits intopatternfly:v6from
kmcfaul:loginfooterlinksitem
Mar 18, 2024
Merged

feat(Login): use children for loginmainfooterlinksitem#10107
tlabaj merged 2 commits intopatternfly:v6from
kmcfaul:loginfooterlinksitem

Conversation

@kmcfaul
Copy link
Contributor

@kmcfaul kmcfaul commented Feb 27, 2024

What: Closes #10100

Removes inner structure of LoginMainFooterLinksItem and updates examples to pass buttons directly.

@patternfly-build
Copy link
Collaborator

patternfly-build commented Feb 27, 2024 •
edited
Loading

Preview: https://patternfly-react-pr-10107.surge.sh

A11y report: https://patternfly-react-pr-10107-a11y.surge.sh

@tlabaj tlabaj requested review from a team, mattnolting, tlabaj and wise-king-sullyman and removed request for a team February 27, 2024 19:54
@kmcfaul
Copy link
Contributor Author

kmcfaul commented Feb 27, 2024 •
edited
Loading

@mcoker There's a pf-v5-svg class that core uses to square up the svg icons that I included here, otherwise the icons are a bit larger and the buttons look rectangular. This class' prefix didn't seem to get updated to v6 though so I'm unsure if I should just remove it?

@mcoker
Copy link
Contributor

mcoker commented Feb 28, 2024

@kmcfaul it looks good to me, though there are react-icons for all of the footer icons - should we use those instead? GoogleIcon, GithubIcon, DropboxIcon, FacebookSquareIcon, GitlabIcon

mcoker
mcoker approved these changes Feb 28, 2024
View reviewed changes
Copy link
Contributor

@mcoker mcoker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Though looks like we can swap the hardcoded SVGs with the react-icons equivalents 👍

tlabaj
tlabaj reviewed Feb 28, 2024
View reviewed changes
Copy link
Contributor

@tlabaj tlabaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a code mod issue for this one?

@kmcfaul kmcfaul mentioned this pull request Feb 28, 2024
Closed
@kmcfaul
Copy link
Contributor Author

kmcfaul commented Feb 28, 2024

Waiting for alpha release before merging.

@tlabaj tlabaj merged commit 922fd90 into patternfly:v6 Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wise-king-sullyman wise-king-sullyman wise-king-sullyman approved these changes

@mcoker mcoker mcoker approved these changes

@tlabaj tlabaj tlabaj approved these changes

@mattnolting mattnolting Awaiting requested review from mattnolting

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@kmcfaul @patternfly-build @mcoker @wise-king-sullyman @tlabaj