Skip to content

Expose label on scope in template #358

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

Merged

Conversation

Liooo
Copy link
Contributor

@Liooo Liooo commented Jun 29, 2016

Gonna fix #355

@ValentinH
Copy link
Member

I just checkout your code and first reverse the logic about the inject-label option (as mentioned in the issue). Then, I commented your watch block and didn't see any issues with the combo label position (even without a $timeout) call.

Here is my translateFn method:

translateFn: function(value, label, which, useCustomTr) {
        useCustomTr = useCustomTr === undefined ? true : useCustomTr;

        var valStr = '',
          getDimension = false,
          dontInjectLabel = label.hasClass('dont-inject-label')

        if (useCustomTr) {
          if (this.options.stepsArray)
            value = this.getStepValue(value);
          valStr = String(this.customTrFn(value, this.options.id, which));
        }
        else {
          valStr = String(value)
        }

        if (label.rzsv === undefined || label.rzsv.length !== valStr.length || (label.rzsv.length > 0 && label.rzsd === 0)) {
          getDimension = true;
          label.rzsv = valStr;
        }

        if(!dontInjectLabel)
          label.html(valStr);
        this.scope[which + 'Label'] = valStr;

        // Update width only when length of the label have changed
        if (getDimension) {
          this.getDimension(label);
        }
      }

Can you try it and tell me if you can still get the positioning problem?

@Liooo
Copy link
Contributor Author

Liooo commented Jun 30, 2016

@ValentinH Did you set the label using angular binding, like {{}} or ng-bind inside the custom template?
That's when the issue is supposed to happen.

@ValentinH
Copy link
Member

Nope I just use the code you added in the demo with the custom directive.

@Liooo
Copy link
Contributor Author

Liooo commented Jun 30, 2016

@ValentinH
Yea it worked. Probably the problem was somehow caused by ng-bind-html.

@codecov-io
Copy link

codecov-io commented Jul 9, 2016

Current coverage is 100%

Merging #358 into master will not change coverage

@@           master   #358   diff @@
====================================
  Files           1      1          
  Lines         838    839     +1   
  Methods         0      0          
  Messages        0      0          
  Branches        0      0          
====================================
+ Hits          838    839     +1   
  Misses          0      0          
  Partials        0      0          

Powered by Codecov. Last updated by 9a9db3e...6256583

@@ -1,4 +1,4 @@
## AngularJS slider directive with no external dependencies
e# AngularJS slider directive with no external dependencies
Copy link
Member

Choose a reason for hiding this comment

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

You probably modify this line by mistake ;)

@Liooo Liooo force-pushed the expose-label-on-scope-in-template branch 4 times, most recently from f1b2658 to 6256583 Compare July 9, 2016 09:04
@Liooo Liooo force-pushed the expose-label-on-scope-in-template branch from 6256583 to 2fa3760 Compare July 9, 2016 09:07
@ValentinH
Copy link
Member

👍 Great job man! It's the best PR I've seen since I maintain this project, everything is here: documentation, tests, etc.

Thanks! :)

@Liooo
Copy link
Contributor Author

Liooo commented Jul 9, 2016

@ValentinH
Can you chekc the pullrequest?

If it's looking good, I'll send one for gh-pages as well.

@Liooo
Copy link
Contributor Author

Liooo commented Jul 9, 2016

@ValentinH Oh that's nice, thanks man :)

@ValentinH ValentinH merged commit dbf7725 into angular-slider:master Jul 9, 2016
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.

Changing the way to set label to increase customizability
3 participants