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

Added OUIA compatibility for Dropdown by quarckster · Pull Request #3135 · patternfly/patternfly-react · GitHub

Added OUIA compatibility for Dropdown - #3135

Merged
redallen merged 1 commit into
patternfly:masterfrom
quarckster:dropdown_ouia
Oct 17, 2019
Merged

Added OUIA compatibility for Dropdown#3135
redallen merged 1 commit into
patternfly:masterfrom
quarckster:dropdown_ouia

Conversation

Copy link
Copy Markdown
Contributor

Part of #2423

What: Added OUIA props to Dropdown as per #2423

karelhala previously approved these changes Oct 14, 2019

quarckster commented Oct 14, 2019
edited
Loading

Copy link
Copy Markdown
Contributor Author

@redallen please help with type matching. Build failed due to this issue.

Copy link
Copy Markdown
Contributor

@quarckster looks like the problem was with incorrect onSelect type. This should do the trick.

diff --git a/packages/patternfly-4/react-core/src/components/Dropdown/Dropdown.tsx b/packages/patternfly-4/react-core/src/components/Dropdown/Dropdown.tsx
index 4cc061c65..86ee33ed3 100644
--- a/packages/patternfly-4/react-core/src/components/Dropdown/Dropdown.tsx
+++ b/packages/patternfly-4/react-core/src/components/Dropdown/Dropdown.tsx
@@ -23,7 +23,7 @@ export interface DropdownProps extends React.HTMLProps<HTMLDivElement> {
   /** Toggle for the dropdown, examples: <DropdownToggle> or <DropdownToggleCheckbox> */
   toggle: React.ReactElement<any>;
   /** Function callback called when user selects item */
-  onSelect?(event: React.SyntheticEvent<HTMLDivElement>): void;
+  onSelect?(event?: React.SyntheticEvent<HTMLDivElement>): void;
   /** Flag to indicate if the first dropdown item should gain initial focus, set false when adding
    * a specific auto-focus item (like a current selection) otherwise leave as true
    */
diff --git a/packages/patternfly-4/react-core/src/components/Dropdown/DropdownWithContext.tsx b/packages/patternfly-4/react-core/src/components/Dropdown/DropdownWithContext.tsx
index fe1ea2218..307e825e8 100644
--- a/packages/patternfly-4/react-core/src/components/Dropdown/DropdownWithContext.tsx
+++ b/packages/patternfly-4/react-core/src/components/Dropdown/DropdownWithContext.tsx
@@ -21,7 +21,7 @@ class DropdownWithContext extends React.Component<DropdownProps & InjectedOuiaPr
     isGrouped: false,
     position: DropdownPosition.left,
     direction: DropdownDirection.down,
-    onSelect: Function.prototype,
+    onSelect: (): void => undefined,
     autoFocus: true,
     ouiaComponentType: 'Dropdown'
   };

Also don't forget to run node_modules/.bin/jest -u packages/patternfly-4/react-core/src/components/ in order to update your snapshots.

Copy link
Copy Markdown
Contributor

@quarckster I believe @karelhala is correct. This mismatching type for onSelect is Function vs (event: SyntheticEvent<HTMLDivElement, Event>) => void.

dlabrecq previously approved these changes Oct 16, 2019
dlabrecq dismissed their stale review October 16, 2019 01:56

Didn't see the mismatching type

quarckster changed the title [WIP] Added OUIA compatibility for Dropdown Added OUIA compatibility for Dropdown Oct 16, 2019

Copy link
Copy Markdown
Collaborator

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

codecov-io commented Oct 16, 2019
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #3135 into master will decrease coverage by <.01%.
The diff coverage is 70%.

@@            Coverage Diff             @@
##           master    #3135      +/-   ##
==========================================
- Coverage   68.98%   68.98%   -0.01%     
==========================================
  Files         858      858              
  Lines       23612    23621       +9     
  Branches     1887     1889       +2     
==========================================
+ Hits        16288    16294       +6     
- Misses       6364     6366       +2     
- Partials      960      961       +1
Flag Coverage Δ
#misc 95.45% <ø> (ø) ⬆️
#patternfly3 69.3% <ø> (ø) ⬆️
#patternfly4 67.93% <70%> (-0.01%) ⬇️
Impacted Files Coverage Δ
...-4/react-core/src/components/Dropdown/Dropdown.tsx 100% <ø> (ø) ⬆️
...re/src/components/Dropdown/DropdownWithContext.tsx 90.32% <70%> (-4.02%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 73c6169...a7390a7. Read the comment docs.

karelhala added the ouia label Oct 16, 2019

Copy link
Copy Markdown
Contributor

Looks like the table snapshots were not updated @quarckster can you run node_modules/.bin/jest -u packages/patternfly-4/ to fix the failing tests as well?

Copy link
Copy Markdown
Contributor Author

@redallen @karelhala @dlabrecq tests passed

redallen merged commit 8a9d853 into patternfly:master Oct 17, 2019

Copy link
Copy Markdown
Collaborator

Your changes have been released in:

  • @patternfly/react-core@3.116.2
  • @patternfly/react-docs@4.14.28
  • @patternfly/react-inline-edit-extension@2.11.97
  • demo-app-ts@3.7.11
  • @patternfly/react-table@2.23.13
  • @patternfly/react-topology@2.9.6
  • @patternfly/react-virtualized-extension@1.2.79

Thanks for your contribution! 🎉

quarckster deleted the dropdown_ouia branch October 17, 2019 14:15
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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants


Back | FazBrowse Home | New Git URL