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

Implement fix for #250 #265

Merged
merged 2 commits into from
Aug 12, 2020
Merged

Implement fix for #250 #265

merged 2 commits into from
Aug 12, 2020

Conversation

Thomas1664
Copy link
Contributor

@Thomas1664 Thomas1664 commented Aug 7, 2020

  • Please check if the PR fulfils these requirements
  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • Unit tests are passing ng test
  • Lint tests are passing ng lint
  • What kind of change does this PR introduce? (Bug fix, feature, docs update, …)
    Bugfix for Scrollbar Hidden not working #250

  • Other information:
    Check if _scrollbarHidden is true. If so, call disableScroll().

@Thomas1664
Copy link
Contributor Author

Not sure why this test fails. Has nothing to do with my changes

@Thomas1664 Thomas1664 closed this Aug 7, 2020
@Thomas1664 Thomas1664 reopened this Aug 7, 2020
@bfwg
Copy link
Owner

bfwg commented Aug 11, 2020

@Thomas1664 The CI is passing on develop so there must be an issue with the change. We shouldn't disable scroll when the drag scroll item is smaller than the container.

expect(window.getComputedStyle(compiled.nativeElement).height).toBe('50px');
expect(window.getComputedStyle(compiled.nativeElement).width).toBe('50px');

@Thomas1664
Copy link
Contributor Author

As you can see in the test logs you have some vulnerabilities in the devDependencies and you use Angular 9 but the latest version is Angular 10. You should consider to update Angular.

Copy link
Owner

@bfwg bfwg left a comment

Choose a reason for hiding this comment

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

Nice work. LGTM 💯

@bfwg bfwg merged commit 30bda21 into bfwg:develop Aug 12, 2020
@bfwg
Copy link
Owner

bfwg commented Aug 12, 2020

9.0.0 beta-4 is out with this change

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.

2 participants