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

fix(button): support setting tab index unless not button and disabled #3240

Merged
merged 1 commit into from Nov 21, 2019

Conversation

@boaz0
Copy link
Member

boaz0 commented Oct 30, 2019

What:

closes #3208

//cc @petli-openshift

@boaz0 boaz0 force-pushed the boaz0:closes_3208 branch from b2de0a8 to e8277df Nov 4, 2019
@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Nov 4, 2019

Codecov Report

Merging #3240 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3240      +/-   ##
==========================================
+ Coverage   67.37%   67.37%   +<.01%     
==========================================
  Files         893      893              
  Lines       24957    24960       +3     
  Branches     2157     2158       +1     
==========================================
+ Hits        16815    16818       +3     
  Misses       7136     7136              
  Partials     1006     1006
Flag Coverage Δ
#misc 95.45% <ø> (ø) ⬆️
#patternfly3 69.28% <ø> (ø) ⬆️
#patternfly4 64.67% <100%> (ø) ⬆️
Impacted Files Coverage Δ
...nfly-4/react-core/src/components/Button/Button.tsx 88.46% <100%> (+0.22%) ⬆️
...3/patternfly-react/src/components/Button/Button.js 100% <0%> (ø) ⬆️

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 fe604f3...7cf7490. Read the comment docs.

@boaz0

This comment has been minimized.

Copy link
Member Author

boaz0 commented Nov 4, 2019

It seems like a problem with the memory size causes pf4 docs build to fail.

In addition, I can build the docs successfully on my machine and can't reproduce it on my machine.

@redallen can you look at it?

@patternfly/react-docs: failed Building static HTML for pages - 70.042s
@patternfly/react-docs: error Building static HTML failed
@patternfly/react-docs: 1 | function _defineProperty(obj, key, value) {
@patternfly/react-docs: > 2 | if (key in obj) {
@patternfly/react-docs: | ^
@patternfly/react-docs: 3 | Object.defineProperty(obj, key, {
@patternfly/react-docs: 4 | value: value,
@patternfly/react-docs: 5 | enumerable: true,
@patternfly/react-docs:
@patternfly/react-docs: WebpackError: Call retries were exceeded

@redallen

This comment has been minimized.

Copy link
Contributor

redallen commented Nov 4, 2019

Yeah, this is an OOM error. I left both -next and -org previews somewhat broken last week and are going to go back and fix them this week. I've rekicked your job for you in the meantime.

@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented Nov 4, 2019

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

@boaz0

This comment has been minimized.

Copy link
Member Author

boaz0 commented Nov 5, 2019

Thanks @redallen !

Copy link
Contributor

tlabaj left a comment

Can you also add the tabIndex prop to the demo-app so we can verify the types.

@tlabaj tlabaj requested a review from kmcfaul Nov 5, 2019
@tlabaj

This comment has been minimized.

Copy link
Contributor

tlabaj commented Nov 19, 2019

@boaz0 Can you update this pR so we can try and get it in? Thanks!

@boaz0

This comment has been minimized.

Copy link
Member Author

boaz0 commented Nov 19, 2019

Yeah sure. No problem.

@boaz0 boaz0 force-pushed the boaz0:closes_3208 branch 2 times, most recently from faf98e6 to 204ff7b Nov 20, 2019
Signed-off-by: Boaz Shuster <boaz.shuster.github@gmail.com>
@boaz0 boaz0 force-pushed the boaz0:closes_3208 branch from 204ff7b to 7cf7490 Nov 20, 2019
@tlabaj
tlabaj approved these changes Nov 20, 2019
Copy link
Contributor

tlabaj left a comment

LGTM

@redallen redallen merged commit 759c8f1 into patternfly:master Nov 21, 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
@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented Nov 21, 2019

Your changes have been released in:

  • @patternfly/react-catalog-view-extension@1.1.34
  • @patternfly/react-core@3.122.4
  • @patternfly/react-docs@4.16.40
  • @patternfly/react-inline-edit-extension@2.13.5
  • demo-app-ts@3.11.4
  • @patternfly/react-integration@3.11.1
  • @patternfly/react-table@2.24.37
  • @patternfly/react-topology@2.11.23
  • @patternfly/react-virtualized-extension@1.3.36

Thanks for your contribution! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.