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

fix(utils): export common utility functions and constants in react-core by priley86 · Pull Request #1232 · patternfly/patternfly-react · GitHub

/ patternfly-react Public

fix(utils): export common utility functions and constants in react-core - #1232

Merged
tlabaj merged 3 commits into
patternfly:masterfrom
priley86:export-utils
Feb 5, 2019
ConversationCommits (3)ChecksFiles changed
Merged

fix(utils): export common utility functions and constants in react-core#1232
tlabaj merged 3 commits into
patternfly:masterfrom
priley86:export-utils

Conversation

priley86 commented Jan 22, 2019

Copy link
Copy Markdown
Member

What:
A minor minor addition to react-core to help out #1227

  • exports some common util functions/constants and convert them to typescript

Additional issues:
none

priley86 requested a review from jschuler January 22, 2019 21:42
priley86 mentioned this pull request Jan 22, 2019
Merged
6 tasks
priley86 requested a review from dlabaj January 22, 2019 21:44

patternfly-build commented Jan 22, 2019

Copy link
Copy Markdown
Collaborator

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

coveralls commented Jan 22, 2019 •
edited
Loading

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 4316

  • 6 of 10 (60.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.01%) to 80.239%
Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/patternfly-4/react-core/src/components/Dropdown/Toggle.js 0 1 0.0%
packages/patternfly-4/react-core/src/components/Chip/Chip.js 2 5 40.0%
Totals
Change from base Build 4313: 0.01%
Covered Lines: 4638
Relevant Lines: 5430
💛 - Coveralls

jschuler previously approved these changes Jan 23, 2019
View reviewed changes
dmiller9911 reviewed Jan 23, 2019
View reviewed changes
Comment thread packages/patternfly-4/react-core/src/index.ts Outdated
@@ -1,4 +1,5 @@
export * from './components';
export * from './layouts';
export * from './internal';

dmiller9911 Jan 23, 2019

Copy link
Copy Markdown
Contributor

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

kind of semantics, but if it is exported it is not really internals anymore. Maybe rename to helpers or something similar?

priley86 Jan 23, 2019

Copy link
Copy Markdown
Member Author

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

i was thinking the same thing this morning actually... i'm up for this if others are.

jschuler Jan 24, 2019

Copy link
Copy Markdown
Contributor

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

helpers sounds good

priley86 Jan 25, 2019

Copy link
Copy Markdown
Member Author

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

ok, works for me. Will update soon.

tlabaj commented Feb 4, 2019

Copy link
Copy Markdown
Contributor

@priley86 looks like this one still needs some updates and conflicts resolved.

priley86 commented Feb 4, 2019

Copy link
Copy Markdown
Member Author

will rebase this and update to helperssoon after #1317 ... would be really helpful to have this.

priley86 commented Feb 5, 2019

Copy link
Copy Markdown
Member Author

*rebased and tested locally. Should be g2g.

kmcfaul previously approved these changes Feb 5, 2019
View reviewed changes
jschuler reviewed Feb 5, 2019
View reviewed changes
Comment thread packages/patternfly-4/react-core/src/helpers/util.ts Outdated
export function debounce(func, wait) {
let timeout;
return (...args) => {
export function debounce(this: any, func: (...args: any[]) => any, wait: number) {

jschuler Feb 5, 2019

Copy link
Copy Markdown
Contributor

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

Method signature has changed, is it needed to use this as first argument?

priley86 Feb 5, 2019

Copy link
Copy Markdown
Member Author

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... will rebase

tlabaj approved these changes Feb 5, 2019
View reviewed changes

tlabaj left a comment

Copy link
Copy Markdown
Contributor

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

LGTM

tlabaj merged commit dc657b9 into patternfly:master Feb 5, 2019
dgutride pushed a commit to dgutride/patternfly-react that referenced this pull request Feb 6, 2019
fix(utils): export common utility functions and constants in react-co…
516ace0 
…re (patternfly#1232)

* fix(utils): export common utility functions and constants used in react-core

* renamed internal to helpers

* update debounce method
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

kmcfaul kmcfaul left review comments
dlabaj Awaiting requested review from dlabaj
+3 more reviewers
dmiller9911 dmiller9911 left review comments
jschuler jschuler approved these changes
tlabaj tlabaj 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.

8 participants

Back | FazBrowse Home | New Git URL