fix(slider): vertical slider in a scrolled container sets value to zero on all clicks

[Thaina]

PR Checklist

Please check that your PR fulfills the following requirements:

• The commit message follows our guidelines
• Tests for the changes have been added or this is not a bug fix / enhancement
• Docs have been added, updated, or were not required

PR Type

What kind of change does this PR introduce?

[ x ] Bugfix
[ x ] Enhancement
[ ] Documentation content changes
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: #11800

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[ ] No

Other information

[Splaktar]

Thank you for your contribution!

The reformatting of the entire spec file makes reviewing this too challenging. Please revert the reformatting so that I can review the actual changes to the tests.

[Splaktar]

When you get a chance, please also squash your commits into a single commit and update the commit message to follow the commit message guidelines.

[Thaina]

When you get a chance, please also squash your commits into a single commit and update the commit message to follow the commit message guidelines.

Are there anyway I could squash commit directly in github?

[Splaktar]

There is no way to squash commits in GitHub directly until merge time.

This guide has some helpful steps for squashing commits using the git command line. It's possible to do via WebStorm's Git UI as well.

[Thaina]

There is no way to squash commits in GitHub directly until merge time.

This guide has some helpful steps for squashing commits using the git command line. It's possible to do via WebStorm's Git UI as well.

Then what's the point for the pull request to squash when you the owner could squashed at merge time? If we require to squash ourself then what is the purpose of merge and squash functionality github has provided? Are there any benefit to enforce a rule that you could only merge what already squashed when github instead want you to squash when you merge?

And you know what. I don't really want to have did all these. It just that I want the bug to be fixed. If you have so many fastidious process to accept pull request like this you could just copied my fixed and put it in the main repo then delete this pull request and I am fine with that

This is not productive for anyone to required the same auto formatting, same environment and same code convention just to fix your bug and don't even know that the approach to fix would satisfied you or not. And I already ask you but you just not answer while I need to fix your bug so I could use the slider in my work. I have frustratingly waste my time wonder why that slider are not working properly so many days already too

[Splaktar]

Then what's the point for the pull request to squash when you the owner could squashed at merge time? If we require to squash ourself then what is the purpose of merge and squash functionality github has provided? Are there any benefit to enforce a rule that you could only merge what already squashed when github instead want you to squash when you merge?

We don't use GitHub's "merge and squash" functionality for a number of reasons

1. it doesn't allow the new commit message to be reviewed
2. some information may be lost as the person who does the squash and merge will need to dig through many commits to deduplicate information
3. I don't actually have access to merge PRs, only to recommend that Googlers merge them

• These Googlers especially don't have time to do 2 because it would mean that it took longer to get in each change, which means fewer changes will get merged

1. Squashing commits, amending commits, and updating PRs is common in open source. Once you learn how to do it, it takes less than 10-15 seconds to squash your commits and update your branch.

We provide guidance for how to update your PR if changes are requested in our Pull Request Guide. This shows how to amend your commit and avoid needing to squash commits.

[Splaktar]

And you know what. I don't really want to have did all these. It just that I want the bug to be fixed. If you have so many fastidious process to accept pull request like this you could just copied my fixed and put it in the main repo then delete this pull request and I am fine with that

Unfortunately, contributing to open source requires more effort than copy/pasting from StackOverflow. This is due to many reasons, but the primary one is that when contributing to open source, you need to work as part of a team. You work is reviewed and feedback needs to be addressed to ensure quality and reduce the chance of regressions. When code gets merged into this project, it gets deployed out to thousands of Google internal and external applications and later is released to NPM where it is then updated into tens of thousands of other applications around the world.

As part of working with a team, you need to be able to take constructive feedback and make changes to your contribution. You also need to be able to participate in design discussions where different approaches are considered. It might take some time before a consensus is reached for the best way to fix an issue while minimizing possible risks and regressions.

[Splaktar]

This is not productive for anyone to required the same auto formatting, same environment and same code convention

You do not need to have the same auto formatting. You should have auto formatting disabled when contributing to this project. Then you can avoid these kinds of issues. Just about every open source project out there will encourage you not to auto format files when you change them. It's best to keep contributions small and focused to allow efficient review and merging.

All major open source projects have some kind of contributor guide and coding conventions that need to be followed. You can find ours here:

• Contribution Guide
• Coding Conventions

just to fix your bug
while I need to fix your bug

I personally did not write all of the code in this repository. It has been a collaborative effort between Google and the community over many years. In this specific case, the code at question is certainly not "mine".

and don't even know that the approach to fix would satisfied you or not. And I already ask you but you just not answer

I am sorry for not responding to your questions immediately, however there are quite a few other priorities that require my attention, especially P1: urgent issues. As this library is not a paid product, we do not offer any kinds of SLA or guaranteed response times to inquiries.

For your specific questions, I looked at them to see if I could provide a quick answer. However, it became clear that I would need to dig in deeper and debug what was going on before I could provide solid guidance on the best solution. I haven't yet had time to investigate this further and I don't have an ETA of when that will happen.

so I could use the slider in my work.

I am glad to hear that you have found this free, open source project useful for commercial applications and for bringing home a paycheck!

I have frustratingly waste my time wonder why that slider are not working properly so many days already too

I am very sorry to hear about how frustrating this issue has proven for you. We've made a number of quality improvements to md-slider over the last couple of years, unfortunately we only recently became aware of this issue (through your issue report, thank you!).

If you need this functionality for an urgent deadline, then I would suggest considering

• using horizontal sliders, which should not have this issue?
• using one of the other AngularJS Material components (input, select)
• finding another slider implementation
• building your own custom slider

To set expectations, getting a change approved, merged, and released from AngularJS Material is a multi-month process in most all cases.

I have put a considerable amount of effort into streamlining this process and improving the documentation over the last 2 years (I'm certainly open to feedback on how the docs can be improved). However, due to requirements that are out of my control, there is still a significant level of effort required to get up to speed with being a regular contributor to this project. I'm happy to discuss process, doc, and convention improvements in more detail on the forums or Gitter if it is of interest to you.

I really do appreciate your contributions and passion for improving this library. Thank you.

[Thaina]

@Splaktar That's my point that I don't really want to be contributor in this repo. I just want the problem to be fixed as fast as possible. And if you could just copy what I have written into the main branch then I wouldn't mind at all, if it could skip all this process I have to do

Actually that's why I use the word your bug because I don't mean the single you. I mean you as the team of angular material, that would include anyone that has contribute to this feature, separate from me who is not but just want to fix this bug

One thing I would like to confess is I have fix this bug by copy the angular.material.js file into my workspace and debug it on my web. Then after I know the problem and test the solution I just edit the file directly in github web editor of this repo. So I don't pull this repo into my machine and github web UI don't have a feature to squash

and don't even know that the approach to fix would satisfied you or not. And I already ask you but you just not answer

I am sorry for not responding to your questions immediately

It was already whole day and while you could write 3 long comments to argue with me 6 hours ago you are still not answer to the question I am asking here #11800 (comment)

I am asking this question since yesterday and I really frustrate that you are not responding to it at all so while I am very desperate to fix it. I have waste more time to debug and research deeper in the md-slider to found out that it was wrong from the start that it use clientrect without ever accounted for scroll in any direction and even I have spun up #11804. But till this time you are still not answer to that question but come response with my argument in this issue instead

And the last thing I want to argue with you is that, github have specifically made squash and merge functionality, instead of making squash functionality to any commit. I could only interpret that github was designed with intention that the puller have to made a decision for squashing the pull request instead of us

If it's true that all opensource repo required that every pull request should be squashed then github should already made that functionality into a made pull request or squash commit button. Why github not made this feature but instead made a squash and merge is then become very mysterious

If I need to squash by myself it means I have to clone this repo then fight with git command line and SSH key just for patching 10 lines in 2 files. If it was a large PR with thousand lines of code then it might be reasonable. But if it just small fix like this it would be ultimately more productive if you could just copy it into master branch instead of this slow PR process

[Splaktar]

That's my point that I don't really want to be contributor in this repo. I just want the problem to be fixed as fast as possible. And if you could just copy what I have written into the main branch then I wouldn't mind at all, if it could skip all this process I have to do

The fastest way to get this problem fixed and merged into AngularJS Material is to collaborate on the solution and go through the required process. If you don't want to do that, then that is no problem. You can just suggest your ideas for fixes in the original issue and leave it for me to address when I get to P2: required issues in a month or two.

[Splaktar]

you are still not answer to the question I am asking here #11800 (comment)

I don't know what the best approach is at this time. If you want this resolved quickly, then I would need you to explore the different approaches, run the unit test suite locally to verify which is less disruptive (fewest tests need to be modified), manually test the changes locally with the slider demos to verify there are no regressions, and then present the proposed fix which you have decided is the best, along with rationale and explanation of the changes.

[Splaktar]

If it's true that all opensource repo required that every pull request should be squashed

That is not true.

It's unlikely that you can contribute more than typo fixes or docs in this repo without doing any local development.

However, I agree that it would be nice if GitHub had a feature where it allowed squashing commits in a PR w/o merging. I just submitted a feature request to GitHub for this functionality.

[Splaktar]

while you could write 3 long comments to argue with me 6 hours ago

I'm sorry if that is the tone that you perceived. I was spending my valuable time providing you with some perspective and trying to help you understand how to contribute. This was with the goal of providing you the tools and understanding needed to get this fix tested and merged as fast as possible.

I'm very interested in onboarding contributors who want to contribute to this project over a long period of time. I'm happy to go out of my way to explain things to them and help them through issues.

[Thaina]

I'm sorry if that is the tone that you perceived

@Splaktar I was fine with any tone you use but I am frustrate by your priority. Because my priority is bug fix. Not even acceptance of pull request. I just want to know the best solution so I could drop the one that unnecessary and the process should be quicker. Or if there are any faster and better solution I will did that

I just submitted a feature request to GitHub for this functionality.

Could I get that too? If it issue then I would upvote it

[Splaktar]

Could I get that too? If it issue then I would upvote it

Unfortunately, it appears to go into a proprietary system and isn't open for upvotes.

[Splaktar]

I've updated the CodePen reproduction of this. I can't seem to reproduce this in the docs-site due to the different levels of scroll containers.

Here's the same CodePen with the fixes from this branch. It does resolve the issue.

[Splaktar]

Note

This PR must use "Squash and merge" and update the squashed commit message to one that follows our guidelines so that we can generate proper a changelog.

[Splaktar]

LGTM other than commit messages and squashing, which I'll try to resolve on merge.

[mmalerba]

LGTM