Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dropdown: allow custom dropdown items #2890

Merged
merged 3 commits into from Sep 10, 2019

Conversation

@jschuler
Copy link
Collaborator

jschuler commented Sep 9, 2019

Closes: #2675

Keyboard navigation triggers on the <li> items now so that any custom node passed into <DropdownItem> can work without trowing errors.

Example:
Screen Shot 2019-09-09 at 1 31 01 PM
Code:

render() {
  const { isOpen } = this.state;
  const dropdownItems = [
    <DropdownItem key="action">
      Default
    </DropdownItem>,
    <DropdownItem>
      <FormattedMessage
        id="welcome"
        defaultMessage={`Hello {name}, you have {unreadCount, number} {unreadCount, plural,
                    one {message}
                    other {messages}
                  }`}
        values={{name: <b>Joachim</b>, unreadCount: 25}}
      />
    </DropdownItem>,
    <DropdownItem key="action" component="button">
      Render as button
    </DropdownItem>,
    <DropdownItem key="link"><Button>Use Button component</Button></DropdownItem>,
    <DropdownItem key="disabled link">
      <Alert title="Use Alert component" />
    </DropdownItem>,
    <DropdownItem key="disabled action" isDisabled component="button">
      Disabled Action
    </DropdownItem>,
    <DropdownSeparator key="separator" />,
    <DropdownItem key="separated link">Separated Link</DropdownItem>,
    <DropdownItem key="separated action" component="button">
      Separated Action
    </DropdownItem>
  ];
  return (
    <IntlProvider locale="en">
      <Dropdown
        onSelect={this.onSelect}
        toggle={<DropdownToggle onToggle={this.onToggle} iconComponent={CaretDownIcon}>Dropdown</DropdownToggle>}
        isOpen={isOpen}
        dropdownItems={dropdownItems}
      />
    </IntlProvider>
  );
}
@jschuler jschuler added the PF4 label Sep 9, 2019
@jschuler jschuler requested review from kmcfaul and tlabaj Sep 9, 2019
@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented Sep 9, 2019

PatternFly-React preview: https://patternfly-react-pr-2890.surge.sh

@tlabaj tlabaj requested a review from jessiehuff Sep 9, 2019
@@ -31,11 +31,11 @@ export interface DropdownMenuItem extends React.HTMLAttributes<any> {
isDisabled: boolean;
disabled: boolean;
isHovered: boolean;
ref: React.RefObject<any>;

This comment has been minimized.

Copy link
@tlabaj

tlabaj Sep 9, 2019

Contributor

Changing this type here wont break anyone?

This comment has been minimized.

Copy link
@kmcfaul

kmcfaul Sep 9, 2019

Contributor

The refs are used internally so I don't think it should

@kmcfaul

This comment has been minimized.

Copy link
Contributor

kmcfaul commented Sep 9, 2019

Waiting on the commit to set the autofocus based on keyboard entry opposed to all the time, but everything else is looking good!

@tlabaj tlabaj requested a review from mcarrano Sep 9, 2019
@tlabaj tlabaj added the ux review label Sep 9, 2019
Copy link
Contributor

jessiehuff left a comment

LGTM - from an accessibility standpoint, keyboard functionality and voiceover work as expected after Jenn and I tested it. :)

Copy link
Member

mcarrano left a comment

This looks fine as far as I can tell.

@mcarrano mcarrano removed the ux review label Sep 9, 2019
Copy link
Contributor

karelhala left a comment

Nice! this is really good adition and will help us a lot!

@jschuler jschuler dismissed stale reviews from karelhala, mcarrano, jessiehuff, and dlabaj via 8a9c177 Sep 10, 2019
Copy link
Contributor

kmcfaul left a comment

Looks great!

Copy link
Contributor

karelhala left a comment

Still looking awesome!

@tlabaj
tlabaj approved these changes Sep 10, 2019
Copy link
Contributor

tlabaj left a comment

LGTM

@tlabaj tlabaj merged commit 9335f73 into patternfly:master Sep 10, 2019
8 checks passed
8 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: build_integration Your tests passed on CircleCI!
Details
ci/circleci: build_pf3_docs Your tests passed on CircleCI!
Details
ci/circleci: build_pf4_docs Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: test_jest_other Your tests passed on CircleCI!
Details
ci/circleci: test_jest_pf4 Your tests passed on CircleCI!
Details
ci/circleci: upload_docs Your tests passed on CircleCI!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.