Skip to content

Conversation

@priley86
Copy link
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

@patternfly-build
Copy link
Collaborator

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

@coveralls
Copy link

coveralls commented Jan 22, 2019

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 Coverage Status
Change from base Build 4313: 0.01%
Covered Lines: 4638
Relevant Lines: 5430

💛 - Coveralls

jschuler
jschuler previously approved these changes Jan 23, 2019
@@ -1,4 +1,5 @@
export * from './components';
export * from './layouts';
export * from './internal';
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

helpers sounds good

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, works for me. Will update soon.

@tlabaj
Copy link
Contributor

tlabaj commented Feb 4, 2019

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

@priley86
Copy link
Member Author

priley86 commented Feb 4, 2019

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

@priley86
Copy link
Member Author

priley86 commented Feb 5, 2019

*rebased and tested locally. Should be g2g.

kmcfaul
kmcfaul previously approved these changes Feb 5, 2019
export function debounce(func, wait) {
let timeout;
return (...args) => {
export function debounce(this: any, func: (...args: any[]) => any, wait: number) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks... will rebase

Copy link
Contributor

@tlabaj tlabaj left a comment

Choose a reason for hiding this comment

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

LGTM

@tlabaj 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
…re (patternfly#1232)

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

* renamed internal to helpers

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

8 participants