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

Allow slider widget to be repositioned #619

Merged
merged 7 commits into from
Oct 13, 2016
Merged

Conversation

manthey
Copy link
Contributor

@manthey manthey commented Sep 26, 2016

It would be nicer if you could change the orientation and which ends had the + and - (or even if they appeared).

You can invoke the slide with the position option, which can contain offsets from the top, left, right, or bottom (or some combination thereof). It can also take a width and height, which are honored, though height needs to be more than 3.5 times the width or stupid things will happen.

This addresses issue #618.

It would be nicer if you could change the orientation and which ends had the + and - (or even if they appeared).

You can invoke the slide with the position option, which can contain offsets from the top, left, right, or bottom (or some combination thereof).  It can also take a width and height, which are honored, though height needs to be more than 3.5 times the width or stupid things will happen.
@manthey
Copy link
Contributor Author

manthey commented Sep 26, 2016

@dorukozturk Give this a try, passing something like {position: {right: 20, top: 20}}

@codecov-io
Copy link

codecov-io commented Sep 26, 2016

Current coverage is 85.39% (diff: 80.00%)

Merging #619 into master will decrease coverage by <.01%

@@             master       #619   diff @@
==========================================
  Files            85         85          
  Lines          8211       8217     +6   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           7012       7017     +5   
- Misses         1199       1200     +1   
  Partials          0          0          

Powered by Codecov. Last update 2ee4693...b75cd89

@manthey manthey changed the title This is the first step to a positionable slider widget. Allow slider widget to be repositioned Oct 10, 2016
@aashish24
Copy link
Member

Looks reasonable to me but I will let @dorukozturk or @jbeezley approve it.

Copy link
Contributor

@jbeezley jbeezley left a comment

Choose a reason for hiding this comment

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

@manthey I fixed a couple of problems that I encountered. Have a look and merge it in if it LGTY.

I noticed two defects in positioning widgets.  If you position a widget, a css property gets set and had no way to get unset.  Therefore, if you set the position via x, y or top, left, then tried to set it by bottom, right, the top, left css would still be present and prevent the desired position.  Further, 'px' was always added to the style, preventing clearing parameters via 'auto' or null (or using expressions like '20%').
@manthey
Copy link
Contributor Author

manthey commented Oct 13, 2016

@jbeezley I added another change. If you'd prefer, I could back it out and add it as a separate PR.

I also notice we don't test setting a widget position after its creation. I'll add a test for that and for the recent change.

@jbeezley
Copy link
Contributor

That's fine with me. Thanks.

@manthey
Copy link
Contributor Author

manthey commented Oct 13, 2016

@jbeezley If this most recent change LGTY let me know (or just merge).

@jbeezley jbeezley merged commit 470d78f into master Oct 13, 2016
@jbeezley jbeezley deleted the positionable-slider-widget branch October 13, 2016 14:15
@manthey manthey mentioned this pull request Oct 13, 2016
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.

4 participants