FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(simple-list): added tabindex, button type, removed hover underline by mcoker · Pull Request #2679 · patternfly/patternfly · GitHub

/ patternfly Public

fix(simple-list): added tabindex, button type, removed hover underline - #2679

Merged
christiemolloy merged 9 commits into
patternfly:masterfrom
mcoker:issue-2674
Feb 7, 2020
ConversationCommits (9)ChecksFiles changed
Merged

fix(simple-list): added tabindex, button type, removed hover underline#2679
christiemolloy merged 9 commits into
patternfly:masterfrom
mcoker:issue-2674

Conversation

mcoker commented Feb 5, 2020

Copy link
Copy Markdown
Contributor

fixes #2674

mcoker mentioned this pull request Feb 5, 2020
Merged

patternfly-build commented Feb 5, 2020

Copy link
Copy Markdown
Collaborator

PatternFly-Next preview: https://patternfly-next-pr-2679.surge.sh

mcarrano approved these changes Feb 5, 2020
View reviewed changes

mcarrano left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks good to me.

mattnolting reviewed Feb 6, 2020
View reviewed changes
Comment thread src/patternfly/components/SimpleList/simple-list-item-link.hbs Outdated
Comment thread src/patternfly/components/SimpleList/simple-list-item-link.hbs Outdated
Comment thread src/patternfly/components/SimpleList/examples/SimpleList.md Outdated
Comment thread src/patternfly/components/SimpleList/examples/SimpleList.md Outdated
Comment thread src/patternfly/components/SimpleList/examples/SimpleList.md Outdated
Comment thread src/patternfly/components/SimpleList/examples/SimpleList.md Outdated
mattnolting approved these changes Feb 6, 2020
View reviewed changes

mattnolting left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LPTM!!

christiemolloy commented Feb 6, 2020

Copy link
Copy Markdown
Member

I find that having the link in the same list as the buttons, can be a weird interaction for the user because the focus ring changes so abruptly on the link. Could you add another example with just links and split the buttons from the links. Or could you change the text that sits inside them to be List item - button or list item - link?

mcoker commented Feb 6, 2020

Copy link
Copy Markdown
Contributor Author

Yeah that's a good idea. I just matched what was in react, but I think you'd either have a list with buttons or links - not both. Will update and ask for the same in the react example.

mcoker and others added 9 commits February 7, 2020 14:36
fix(simple-list): added tabindex, button type, removed hover underline
08a3fcd
Update src/patternfly/components/SimpleList/simple-list-item-link.hbs
0ae5c81 
Co-Authored-By: Matt Nolting <matthew.nolting@gmail.com>
Update src/patternfly/components/SimpleList/simple-list-item-link.hbs
7a07bc4 
Co-Authored-By: Matt Nolting <matthew.nolting@gmail.com>
Update src/patternfly/components/SimpleList/examples/SimpleList.md
94c3422 
Co-Authored-By: Matt Nolting <matthew.nolting@gmail.com>
Update src/patternfly/components/SimpleList/examples/SimpleList.md
06504b2 
Co-Authored-By: Matt Nolting <matthew.nolting@gmail.com>
Update src/patternfly/components/SimpleList/examples/SimpleList.md
7bc9f5f 
Co-Authored-By: Matt Nolting <matthew.nolting@gmail.com>
Update src/patternfly/components/SimpleList/examples/SimpleList.md
0f63b31 
Co-Authored-By: Matt Nolting <matthew.nolting@gmail.com>
fix(simplelist): missed a spot
aac66e8
chore(simple-list): create separate link example
1004f96
christiemolloy approved these changes Feb 7, 2020
View reviewed changes

christiemolloy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

thanks for making those updates!!

christiemolloy merged commit c3efeee into patternfly:master Feb 7, 2020

redallen commented Feb 7, 2020

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 2.59.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

mattnolting mattnolting approved these changes
+2 more reviewers
mcarrano mcarrano approved these changes
christiemolloy christiemolloy approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL