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.

Sign up for GitHub

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

Jump to bottom

fix: Padding of leaf nodes for all levels #389

Merged
merged 3 commits into from
Jul 7, 2020
Merged

fix: Padding of leaf nodes for all levels #389

merged 3 commits into from
Jul 7, 2020

Conversation

non-engineer-dev
Copy link
Contributor

What does it do?

Fixed padding (indentation) of leaf nodes for all levels

paddingIncorrect

paddingCorrect

codeclimate[bot]
codeclimate bot reviewed Jul 1, 2020
View reviewed changes
src/tree-node/index.js Outdated Show resolved Hide resolved
codeclimate[bot]
codeclimate bot reviewed Jul 5, 2020
View reviewed changes
src/tree-node/index.js Outdated Show resolved Hide resolved
src/tree-node/toggle.js Outdated Show resolved Hide resolved
src/tree-node/toggle.js Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Jul 5, 2020 •
edited
Loading

Pull Request Test Coverage Report for Build 1432

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.005%) to 93.995%
Totals Coverage Status
Change from base Build 1422: 0.005%
Covered Lines: 601
Relevant Lines: 622
💛 - Coveralls

codeclimate[bot]
codeclimate bot reviewed Jul 7, 2020
View reviewed changes
src/tree-node/toggle.js Outdated
const toggleCx = ['toggle', expanded && 'expanded', !expanded && 'collapsed'].filter(Boolean).join(' ')

if (isLeaf) {
return (<i role="button" tabIndex={-1} className={toggleCx} style={{ visibility: 'hidden' }} aria-hidden />)
Copy link

Choose a reason for hiding this comment

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

Replace (<i·role="button"·tabIndex={-1}·className={toggleCx}·style={{·visibility:·'hidden'·}}·aria-hidden·/>) with <i·role="button"·tabIndex={-1}·className={toggleCx}·style={{·visibility:·'hidden'·}}·aria-hidden·/>

codeclimate[bot]
codeclimate bot reviewed Jul 7, 2020
View reviewed changes
Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 25768 lines exceeds the maximum allowed for the inline comments feature.

@codeclimate
Copy link

codeclimate bot commented Jul 7, 2020

Code Climate has analyzed commit b431bb9 and detected 0 issues on this pull request.

View more on Code Climate.

@mrchief
Copy link
Collaborator

mrchief commented Jul 7, 2020

@all-contributors please add @akarshjairaj for code

@allcontributors allcontributors bot mentioned this pull request Jul 7, 2020
Merged
@allcontributors
Copy link
Contributor

@mrchief

I've put up a pull request to add @akarshjairaj! 🎉

@mrchief mrchief changed the title Fixed padding of leaf nodes for all levels fix: Padding of leaf nodes for all levels Jul 7, 2020
@mrchief
Copy link
Collaborator

mrchief commented Jul 7, 2020

@akarshjairaj for future, you may want to sign your commits: https://docs.github.com/en/github/authenticating-to-github/about-commit-signature-verification

@mrchief mrchief merged commit 5cdd749 into dowjones:develop Jul 7, 2020
@github-actions
Copy link

github-actions bot commented Jul 7, 2020

🎉 This PR is included in version 2.3.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

@aigyptios aigyptios mentioned this pull request Jul 8, 2020
Closed
@mobilutz
Copy link
Contributor

@akarshjairaj & @mrchief Did you add package-lock.json on prupose or accidentally within this PR?

I see quite a huge dependency change with the upgrade from 2.3.3 to 2.3.4 which I can only trace back to this change.

@non-engineer-dev
Copy link
Contributor Author

@mobilutz Apologies. It was an accident and wasn't done on purpose.
@mrchief Let me know how we should go ahead with this.

@mrchief
Copy link
Collaborator

mrchief commented Jul 13, 2020

Let's get rid of it. My bad for not catching it.

@mrchief
Copy link
Collaborator

mrchief commented Jul 13, 2020

In addition to removing it, I think we should add a gitignore entry to be safe in future.

see quite a huge dependency change with the upgrade from 2.3.3 to 2.3.4

@mobilutz are there any CI checks we can put in place to detect things like this?

@non-engineer-dev non-engineer-dev mentioned this pull request Jul 13, 2020
Closed
@non-engineer-dev
Copy link
Contributor Author

@mrchief Created a new PR. Apologies for the mistake.

@mrchief
Copy link
Collaborator

mrchief commented Jul 13, 2020

@akarshjairaj thanks. I'll take a look. Also, please stop being sorry. We all make mistakes.😄

@mobilutz mobilutz mentioned this pull request Jul 13, 2020
Merged
10 tasks
@mobilutz
Copy link
Contributor

@akarshjairaj You really don't need to apologise multiple times - we all did things like this.

@mrchief I opened a PR to remove the package-lock.json and adding tests to check that this does not happen again:
#394

Please review.

@non-engineer-dev
Copy link
Contributor Author

@mrchief @mobilutz I got a little scared it might have had impacted multiple things! But, yes, lesson learnt. :)

m4theushw pushed a commit to m4theushw/react-dropdown-tree-select that referenced this pull request Sep 20, 2020
@non-engineer-dev @m4theushw
fix: Padding of leaf nodes for all levels (dowjones#389)
1f1bbf1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codeclimate codeclimate[bot] codeclimate left review comments

@mrchief mrchief mrchief approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@non-engineer-dev @coveralls @mrchief @mobilutz @headrohit96