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 #5760: only show ui-grid-scrollbar-placeholder if there's a horizontal scrollbar in another container #6244

Closed
wants to merge 7 commits into from

Conversation

jeffantosh
Copy link

Basically adding a check to see if the sum of all of the widths containers in the grid exceeds the viewport size. If true and if horizontal scrolling is not disabled, then show the ui-grid-scrollbar-placeholder div.

@jeffantosh jeffantosh changed the title fix #5760 (only show ui-grid-scrollbar-placeholder if there's a horizontal scrollbar in another container) fix(5760): only show ui-grid-scrollbar-placeholder if there's a horizontal scrollbar in another container Jun 2, 2017
@jeffantosh jeffantosh changed the title fix(5760): only show ui-grid-scrollbar-placeholder if there's a horizontal scrollbar in another container fix #5760: only show ui-grid-scrollbar-placeholder if there's a horizontal scrollbar in another container Jun 2, 2017
Copy link
Member

@mportuga mportuga left a comment

Choose a reason for hiding this comment

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

Could you add some unit tests to cover all possible scenarios for this? I want to make sure that no one breaks this function in the future.

@jeffantosh
Copy link
Author

I don't know enough about the inner workings of ui grid to make these unit tests work. I also was unable to get the unit tests to even execute on my machine.

PhantomJS 2.1.1 (Linux 0.0.0) GridRenderContainer factory needsHScrollbarPlaceholder should return false if the container does not have the horizontal scrollbar enabled FAILED
Expected 0 to be false.
/home/travis/build/angular-ui/ui-grid/test/unit/core/factories/GridRenderContainer.spec.js:242:53
PhantomJS 2.1.1 (Linux 0.0.0) GridRenderContainer factory needsHScrollbarPlaceholder should return true if another container does have a horizontal scrollbar and the current container does not have the horizontal scrollbar enabled AND the total canvas width is greater than the grid width FAILED
Expected false to be true.
/home/travis/build/angular-ui/ui-grid/test/unit/core/factories/GridRenderContainer.spec.js:246:53

@mportuga
Copy link
Member

Closing since we are unable to fix the unit tests at this time.

@mportuga mportuga closed this Nov 13, 2017
@piller187
Copy link

Just wondering what's the deal with this issue as it's still an issue it seems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants