Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
UI Slider Widget #7116
base: main
Are you sure you want to change the base?
UI Slider Widget #7116
Changes from 20 commits
71056a3
d607e6b
83f2417
6da0e41
3500039
d823b1b
f68eb33
6ed303e
23a22ae
ac8f4dc
c19e507
2c432bf
a038156
13aff11
52bfd93
7b5c9be
ae51b05
4616e76
3957bff
7394000
e5ad68e
b208c9f
c35da65
0a013ac
625673f
841350f
8837a50
65bb68c
acb99e3
1d2bb26
db1564b
95506a2
e98d5a7
48986d1
92ff4f2
8084661
5e51efa
3cc3743
af5f7e7
f47bcfc
d35a9b5
5f6c73f
dd7371b
8d13a1a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Could we move this over to the
slider.rs
and have everything be contained there?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.
Hmm. Honestly think we should do the same with
ButtonBundle
,TextBundle
andImageBundle
and just keep the core building blocks for UI innode_bundles.rs
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 kinda like having every bundles in a single file and not need to search around to find each bundle definition, but I understand that this won't scale super well.
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 don't know if I want to do this in this PR. It was originally supposed to be a simple slider widget, but it's slowly turning into a widget rework.
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.
Yeah, it's a case of it being the first "somewhat" advanced widget and will define the structure for all widgets. But I do agree with @IceSentry that we should split it apart into 3 PRs:
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.
Let's continue the
WidgetPlugin
discussion over here, if necessary #7190There 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.
Same here