feat: Added a range #173
feat: Added a range #173
Conversation
|
Thank you for the PR! 😄 🎉 I've got a busy week coming up but will try to get around to reviewing it in the next few days. A range component is definitely something I'd like to get in! Btw, does this support vertical orientation? Lastly a quick note, please follow the commit message rules in the contributing guide, as I may be enabling automatic releasing in the future. For now though when I merge this I will squash your commits. |
|
Hi @edcarroll ! Yes I'm embarrassed to say that I read the section about commits after I already committed my work and I cant change the messages after the fact, I'l make sure to work accordingly in future commits. :) |
|
No worries re commit messages, have done the same a few times myself!! So I had a browse through the repo you're porting and there is a link to a vertical slider implementation - I wonder if that could be a starting point? I'm very keen to get horizontal and vertical functionality in before merging, as I'd imagine this would be one of the major initial feature requests! Things have been super busy at work, but I should be able to look at this day after tomorrow, or failing that the start of the weekend. Thank you for your patience 😄 |
|
I see, I'll try and take a look, I have a busy weekend but I might be able to implement this over the weekend. |
|
Thank you for this 😄 Btw, I did some digging earlier and found this pull request, in which the demo has stuff like vertical sliders and ticks for steps - I wonder if it would be easier to incorporate the CSS from this PR? Then things should be a bit easier and not require any custom CSS from this library... What are your thoughts? |
|
Wow, that demo is really well done, it adds a lot of features, I'l take a stab at it at the weekend. Now looking at that PR, it looks like the entire CSS is LESS and we cant really include it as a link without including it inside the project, how would you like to advance with this ? |
There was a problem hiding this comment.
Few initial comments. Am encountering a bug in which the thumb stays attached to the mouse after clicking off and won't go away.
I'd really like to include the functionality from the linked PR before merging this (specifically 2 thumbs, vertical & snapping to steps) - what are your thoughts there?
Thanks, and apologies for the slowness, trying to finish up a work project that is becoming a bit all encompassing!
| @@ -1,12 +1,8 @@ | |||
| import { NgModule } from "@angular/core"; | |||
| import { Routes, RouterModule } from "@angular/router"; | |||
| import { GettingStartedPage } from "./pages/getting-started/getting-started.page"; | |||
|
|
|||
| // Collections | |||
There was a problem hiding this comment.
Leave the import structure as it was
| @@ -0,0 +1,43 @@ | |||
| <demo-page-title> | |||
| <div header>Rating</div> | |||
| <demo-page-title> | ||
| <div header>Rating</div> | ||
| <div sub-header> | ||
| <p>A rating indicates user interest in content</p> |
There was a problem hiding this comment.
A range allows you to select a range of values?
| CSS alongside Semantic UI's. | ||
| </p> | ||
| <div class="ui segment"> | ||
| <demo-codeblock language="markup" [src]="cssInclude"></demo-codeblock> |
There was a problem hiding this comment.
This repo has the code in it... I wonder if there is a way of easily pulling out the theming files?
| @Component({ | ||
| selector: "sui-range", | ||
| template: ` | ||
| <div class="ui range" [ngClass]="{'disabled': isReadonly}" > |
| selector: "sui-range", | ||
| template: ` | ||
| <div class="ui range" [ngClass]="{'disabled': isReadonly}" > | ||
| <div #inner class="inner" (mousedown)="onDown($event, false)" (touchstart)="onDown($event, true)"> |
There was a problem hiding this comment.
Worth moving some of this complexity into a thumb.ts file?
| }) | ||
| export class SuiRange implements OnInit { | ||
| @Input() public max:number = 100; | ||
| @Input() public min:number = 0; |
| export class SuiRange implements OnInit { | ||
| @Input() public max:number = 100; | ||
| @Input() public min:number = 0; | ||
| @Input() public step:number = 1; |
There was a problem hiding this comment.
Same as above, won't write this for all of them 😛
| const upEventName = isTouch ? "touchend" : "mouseup"; | ||
| const moveEventName = isTouch ? "touchmove" : "mousemove"; | ||
|
|
||
| const removeOnUp = this._renderer.listen("window", upEventName, () => this.onUp()); |
| @@ -1,12 +1,5 @@ | |||
| import { NgModule } from "@angular/core"; | |||
|
|
|||
| // Collections | |||
| import { | |||
|
I've linked the fork that the PR is coming from, which has the Range CSS & overrides to allow themeing support. I'm trying to think of how best to enable extra builds (perhaps a custom module just for theming?) |
|
I've had a chat with a few people about this and I think going down the route of having a separate module that packages the CSS is the way to go, so I'll be working on that in a few weeks. This will work excellently with this PR because it means we can include the external styles for now, but then when the builder is in place it will be automatically included for people so they don't need to worry about Range or Datepicker CSS 😄 |
|
Hey @gotenxds just wanted to check in with you see how you were getting on 😄 |
|
Hi @edcarroll sorry, I've been really swamped lately so I've had like 0 time to work on anything, I'l try and get back to this this weekend. |
|
@gotenxds no worries at all, no rush (work is certainly more important!) - just wanted to check in 😄 |
Hi, I needed a range component for angular with semantic ui and it seems like it will take a while (since 2014)
for semantic-ui to get this done so I thought I'l just make one for now and we can change it when the time comes.
This is a port from https://github.com/tyleryasaka/semantic-ui-range
The same repo semantic-ui is considering working with so it should make it easier to change in the future.
This supports min and max, a step value to determine the amount to skip, a readonly that disables the ui and stops incoming value changes.
It supports touch events as well.