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.

Sign up for GitHub

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

Jump to bottom

Add a widget gallery example. #1010

Merged
merged 9 commits into from
Aug 19, 2020
Merged

Add a widget gallery example. #1010

merged 9 commits into from
Aug 19, 2020

Conversation

richard-uk1
Copy link
Collaborator

I thought a widget gallery might be useful for people to see what widgets are available (like Flutter).

The Grid used to display them is also probably useful in other situations.

@luleyleo luleyleo added the S-waiting-on-author waits for changes from the submitter label Jun 1, 2020
@richard-uk1 richard-uk1 force-pushed the gallery branch 3 times, most recently from b56724e to 7d441dd Compare June 2, 2020 12:02
@luleyleo luleyleo added S-needs-review waits for review and removed S-waiting-on-author waits for changes from the submitter labels Jun 2, 2020
@richard-uk1
Copy link
Collaborator Author

Friendly bump

@richard-uk1
Copy link
Collaborator Author

Ping @cmyr I think something like this to show off widgets would be cool. :)

The main review task is to have a look at the example when run and see if it's something you would like to add.

@cmyr
Copy link
Member

cmyr commented Jun 11, 2020

right yea I was looking at this yesterday and then got distracted wanting to clean up assets in examples; see #1027.

I'll get a review in soonish!

@richard-uk1
Copy link
Collaborator Author

Ooh ok I do have an asset so maybe I need to move it.

cmyr
cmyr reviewed Jun 11, 2020
View reviewed changes
Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

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

Looks good, a few little nits inline.

high-level stuff: I might include the image and svg widgets, maybe just displaying some text like "requires the "image" feature" if those features aren't available; and for those I would reuse existing assets, because why not.

I'm not sure I would include Painter, and if I did I would use it to show something particularly weird, like a clipped gradient or some other sort of drawing that would be a reason to use it.

Oh also there's now a spinner widget, which it might be worth including?

druid/examples/widget_gallery.rs Outdated Show resolved Hide resolved
druid/examples/widget_gallery.rs Outdated Show resolved Hide resolved
druid/examples/widget_gallery.rs Outdated Show resolved Hide resolved
@richard-uk1
Copy link
Collaborator Author

I added a spinner example. :)

@cmyr
Copy link
Member

cmyr commented Jun 12, 2020

huh, that CI failure is weird; seems to happen across a bunch of crates on the current nightly, and unrelated to this PR; will look to open an issue somewhere if this persists over the next few days.

@cmyr
Copy link
Member

cmyr commented Jun 12, 2020

Okay only last thing, per my previous comment:

I'm not sure I would include Painter, and if I did I would use it to show something particularly weird, like a clipped gradient or some other sort of drawing that would be a reason to use it.

That is: we have an image widget, which should be used to display images; painter is more about implementing some sort of custom reusable drawing code. I could imagine it not being included here (since this is mostly about control/ui widgets?) but if it is I would use an example that was a bit more custom; and if I'm including Painter I would probably also include Image and Svg (with placeholders saying 'turn on this feature' if the feature flag isn't set).

@cmyr cmyr added S-waiting-on-author waits for changes from the submitter and removed S-needs-review waits for review labels Jun 18, 2020
@xStrom xStrom added the examples concerns examples label Jun 20, 2020
@richard-uk1
Copy link
Collaborator Author

I'm sorry I will get round to this eventually.

@xStrom
Copy link
Member

xStrom commented Jun 20, 2020

I'm sorry I will get round to this eventually.

No rush, whenever is convenient, nothing is blocked by this PR.

richard-uk1 and others added 6 commits June 29, 2020 20:41
@richard-uk1
Add a widget gallery example.
3a28e0a 
The `Grid` used to display them is also probably useful in other
situations.
@richard-uk1
Move xi.image to assets.
9d8cabd
@richard-uk1 @cmyr
Update druid/examples/widget_gallery.rs
56d918a 
Co-authored-by: Colin Rofls <[email protected]>
@richard-uk1
Fix nits
51ac30f
@richard-uk1
Add spinner example.
d3fe591
@richard-uk1
Multiple changes:
607bcaa 
 - Use `im::Vector<_>` rather than `Arc<Vec<_>>`
 - Add `Image` and `Svg` examples.
 - Use some more fancy stuff in the `Painter` example.
cmyr
cmyr reviewed Jul 14, 2020
View reviewed changes
Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

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

@derekdreery apologies for letting this sit.

A few little inline comments.

One other thought arises: this almost feels like it should be a multi-file example, since there's so much going on in this file? but we haven't done that yet, so I'm okay with not doing it for this.

druid/examples/widget_gallery.rs Outdated Show resolved Hide resolved
druid/examples/widget_gallery.rs Outdated Show resolved Hide resolved
druid/examples/widget_gallery.rs Outdated Show resolved Hide resolved
@richard-uk1
Copy link
Collaborator Author

The grid widget from this example might be useful to others, should I put it in druid itself?

@cmyr
Copy link
Member

cmyr commented Jul 20, 2020

So that's a good question. I do think we will want a more robust 'collection view' widget, but I don't know exactly what that should look like, but I'd want it to cover a pretty wide variety of use cases. Doing this though probably requires us to figure out a general approach to collections in general, and for that I would start by trying to clean up and improve the API of the List widget.

@richard-uk1
Copy link
Collaborator Author

Should be ready for review again.

@cmyr cmyr self-assigned this Aug 17, 2020
cmyr
cmyr approved these changes Aug 18, 2020
View reviewed changes
Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

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

Okay, thanks @derekdreery and sorry for the long wait!

@richard-uk1 richard-uk1 merged commit 80afcdf into linebender:master Aug 19, 2020
@richard-uk1
Copy link
Collaborator Author

no worries :)

@richard-uk1 richard-uk1 deleted the gallery branch August 19, 2020 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@luleyleo luleyleo luleyleo left review comments

@xStrom xStrom xStrom left review comments

@cmyr cmyr cmyr approved these changes

Assignees

@cmyr cmyr

Labels
examples concerns examples S-waiting-on-author waits for changes from the submitter
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@richard-uk1 @cmyr @xStrom @luleyleo