-
Notifications
You must be signed in to change notification settings - Fork 79
GradientSlider component creation #769
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
base: main
Are you sure you want to change the base?
Conversation
|
This is so cool! 🦙❤️ Thanks for thinking of this and putting this together, excited to go peek at how you've put it together in a minute. Did a quick check on the CI, seems like some of the namespaces didn't get globalized properly for WinUI 2: |
michael-hawker
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some initial comments, this is an amazing start! Really interesting use of the Canvas. I'll have to pull it down and play with it a bit more directly.
I'm also wondering since I added Thumbs to the ResizeAdorner, and you have them here if there's any commonalities and if we just need a general Thumb base class like I did for the Sizers (which could honestly maybe re-use that as well, I mean I basically took that code and made it two-dimensional for the Adorner thumb).
| <controls:GradientSlider.GradientStops> | ||
| <GradientStopCollection> | ||
| <GradientStop Offset="0" Color="#FF0000" /> | ||
| <GradientStop Offset="0.3" Color="#00FF00" /> | ||
| <GradientStop Offset="0.66" Color="#0000FF" /> | ||
| <GradientStop Offset="1" Color="#FF0000" /> | ||
| </GradientStopCollection> | ||
| </controls:GradientSlider.GradientStops> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if the developer doesn't specify any stops at the start? Could be good to have another example where it's bound to a collection (to show binding) but that collection is also empty to start?
| <Import Project="$(ToolingDirectory)\ToolkitComponent.SourceProject.props" /> | ||
| </Project> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| <Import Project="$(ToolingDirectory)\ToolkitComponent.SourceProject.props" /> | |
| </Project> | |
| <Import Project="$(ToolingDirectory)\ToolkitComponent.SourceProject.props" /> | |
| <PropertyGroup> | |
| <PackageId>$(PackageIdPrefix).$(PackageIdVariant).Controls.$(ToolkitComponentName)</PackageId> | |
| </PropertyGroup> | |
| </Project> |
We need the Controls added to the package, right @Arlodotexe? (Remind me again why we don't grab this from the csproj name?)
| if (slider._isDragging) | ||
| return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So if a stop is somehow programmatically added while the slide is in use, the slider will never get updated and be out-of-sync? We probably need to call RefreshThumbs or some other sync method when the drag is completed, eh?
| <FlyoutBase.AttachedFlyout> | ||
| <Flyout Placement="Bottom"> | ||
| <muxc:ColorPicker x:Name="PART_ColorPicker" /> | ||
| </Flyout> | ||
| </FlyoutBase.AttachedFlyout> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm always leery of adding intertwined children/parents, but I think in this case, it'd be nice if there was a single attachedflyout was part of the parent control and the child invoked that atop itself? Obviously, requires a bit more coordination, but...
Then I think it gets easier to expose the properties of the ColorPicker itself in one place on the GradientSlider. i.e. if they want to show a specific color palette or other things instead, they just have to restyle the parent control which is much simpler than the thumb.
Should we also use the WCT ColorPicker by default instead? Or should we expose a ColorPicker property on the slider so the developer themselves provide the picker they want to use configured exactly how they want (we can have a style value set with a default basic one in case they don't provide one)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would think, double-tapping on the slider should be the mechanism to add a new stop, eh? @niels9001 any suggestions on input here?
I'm trying to think of how you'd remove a stop, would it be dragging it further off some amount the end of the slider?
The only other scenario I'm thinking of here that'd be more complex to add is if you wanted a specific input box for the offset vs. drag input. (It could be nice in either case as you're dragging to see the offset value as a float [0-1] and/or percentage maybe?) Maybe it's not something we provide by default out-of-the-box, but something we should have a sample on how to extend the control to enable. i.e. could the flyout be customized to contain more than just a colorpicker, would we have to search the flyout for the colorpicker instance then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Co-authored-by: Michael Hawker MSFT (XAML Llama) <[email protected]>

Introduces the first draft of the
GradientSlidercomponent.Resolves: #768
GradientSlider.mp4