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

feat(OptionsMenu): Options Menu Component Typescript Conversion #2002

Merged

Conversation

@nicolethoen
Copy link
Contributor

nicolethoen commented May 14, 2019

#2024

Includes demo and integration tests

@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented May 14, 2019

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented May 14, 2019

Codecov Report

Merging #2002 into master will decrease coverage by 0.35%.
The diff coverage is 58.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2002      +/-   ##
==========================================
- Coverage   81.09%   80.73%   -0.36%     
==========================================
  Files         643      642       -1     
  Lines        7706     7777      +71     
  Branches      451      504      +53     
==========================================
+ Hits         6249     6279      +30     
- Misses       1257     1258       +1     
- Partials      200      240      +40
Flag Coverage Δ
#patternfly3 84.88% <ø> (ø) ⬆️
#patternfly4 76.25% <58.33%> (-0.64%) ⬇️
#patternflymisc 95.68% <ø> (ø) ⬆️
Impacted Files Coverage Δ
...mponents/OptionsMenu/OptionsMenuToggleWithText.tsx 36.36% <36.36%> (ø)
...e/src/components/OptionsMenu/OptionsMenuToggle.tsx 43.47% <43.47%> (ø)
...ore/src/components/OptionsMenu/OptionsMenuItem.tsx 52.38% <52.38%> (ø)
...rc/components/OptionsMenu/OptionsMenuItemGroup.tsx 57.14% <57.14%> (ø)
...rc/components/OptionsMenu/OptionsMenuSeparator.tsx 85.71% <85.71%> (ø)
...ct-core/src/components/OptionsMenu/OptionsMenu.tsx 85.71% <85.71%> (ø)
... and 2 more

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 6c04803...0bfbbe2. Read the comment docs.

Copy link
Member

seanforyou23 left a comment

Awesome work! Can we use git mv to rename the primary component files instead of deleting and recreating them? If we can do this, we'll retain the ability to trace the history of these modules going forward which is important for tracking down when certain changes were made, by who, and why. Looks like you've already done this for several of them.

@nicolethoen nicolethoen force-pushed the nicolethoen:options_menu_typescript_conversion branch 2 times, most recently from 87011e1 to 4d64cd6 May 15, 2019
@nicolethoen nicolethoen changed the title feat(OptionsMenu): Options Menu Component Typescript Conversion (WIP) feat(OptionsMenu): Options Menu Component Typescript Conversion May 15, 2019
@nicolethoen

This comment has been minimized.

Copy link
Contributor Author

nicolethoen commented May 15, 2019

I'm noticing the OptionsMenuWithText component is having its properties listed twice in the documentation - and the second time they are printed, the descriptions and default values are missing. Does anyone know why that would be happening? @redallen @dgutride

@rachael-phillips

This comment has been minimized.

Copy link
Contributor

rachael-phillips commented May 17, 2019

@dgutride @redallen were we able to address @nicolethoen 's comment above?

@nicolethoen

This comment has been minimized.

Copy link
Contributor Author

nicolethoen commented May 17, 2019

@dgutride @redallen were we able to address @nicolethoen 's comment above?

I think that quirk resolved itself. :)

@dlabaj dlabaj requested a review from tlabaj May 21, 2019
@nicolethoen nicolethoen force-pushed the nicolethoen:options_menu_typescript_conversion branch from f5ec6ca to 2453d71 May 22, 2019
Copy link
Contributor

dlabaj left a comment

Lgtm

@nicolethoen nicolethoen force-pushed the nicolethoen:options_menu_typescript_conversion branch from 2453d71 to f56383e May 28, 2019
@nicolethoen nicolethoen force-pushed the nicolethoen:options_menu_typescript_conversion branch from f56383e to fd9f04a May 29, 2019
@nicolethoen nicolethoen force-pushed the nicolethoen:options_menu_typescript_conversion branch from fd9f04a to 26130de May 29, 2019
Copy link
Contributor

redallen left a comment

Awesome, thanks!

@redallen redallen merged commit 402718b into patternfly:master May 30, 2019
2 checks passed
2 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants
You can’t perform that action at this time.