Skip to content

Conversation

dassaptorshi
Copy link
Contributor

If possible please merge it to 5.4.3 as we have started using that version of yours.

Please kindly approve

@codecov-io
Copy link

codecov-io commented Aug 25, 2016

Current coverage is 100% (diff: 100%)

Merging #405 into master will not change coverage

@@           master   #405   diff @@
====================================
  Files           1      1          
  Lines         852    854     +2   
  Methods         0      0          
  Messages        0      0          
  Branches        0      0          
====================================
+ Hits          852    854     +2   
  Misses          0      0          
  Partials        0      0          

Powered by Codecov. Last update b0c190b...2c29a06

@ValentinH
Copy link
Member

Please describe the purpose of this PR and provide a demo.

@dassaptorshi
Copy link
Contributor Author

This PR is to configure the directive so the limit labels don't hide when we reach a particular limit towards floor or ceil value.

Demo

@dassaptorshi
Copy link
Contributor Author

Hi Valentin,
How long do you think it may take to merge the pull request?

Thanks in advance

src/rzslider.js Outdated
}

this.shFloorCeil();
if(!this.optionsshowLimitLabels){
Copy link
Member

Choose a reason for hiding this comment

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

this is wrong

@ValentinH
Copy link
Member

I'm sorry, I'm currently on holidays so I don't have access to my computer and won't be able to release it anyway...

Here are a few remarks:

  • I would prefer to replace the showLimitLabels by something like autoHideLabels: true by default so its purpose is more obvious.
  • other comments are on the lines directly...

@dassaptorshi
Copy link
Contributor Author

Done!

@ValentinH
Copy link
Member

The dist file is still incorrect, you should run grunt before commiting...

@dassaptorshi
Copy link
Contributor Author

Done!

@ValentinH ValentinH merged commit 1070cd2 into angular-slider:master Sep 6, 2016
@ValentinH
Copy link
Member

ValentinH commented Sep 6, 2016

Merged and released under 5.5.0. I have just rename the new option to autoHideLimitLabels since it makes more sense.

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