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

Listiter impl #1641

Merged
merged 18 commits into from
Mar 12, 2021
Merged

Listiter impl #1641

merged 18 commits into from
Mar 12, 2021

Conversation

Lejero
Copy link
Contributor

@Lejero Lejero commented Mar 10, 2021

Added ListIter implementations for HashMap and OrdMap. Still unsure if handling key modifications the best way, but values should be good. (This is my first PR here)

@cmyr cmyr added the S-needs-review waits for review label Mar 10, 2021
@Lejero
Copy link
Contributor Author

Lejero commented Mar 11, 2021

I'm not very familiar with web assembly but I can't get the test to run on my computer. Not sure if there's additional setup I need to do it or possibly its having trouble on an M1-mac which I'm using.

@futurepaul
Copy link
Collaborator

I believe you're missing an entry in druid/examples/web/src/lib.rs for your example. Maybe that will solve the test failure?

(I abandoned a similar PR a while back glad to see this being picked up!)

@Lejero
Copy link
Contributor Author

Lejero commented Mar 11, 2021

Yes, that was exactly it. I just pushed the change adding to the web examples and all the tests ran successfully. Thanks.

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.

A couple little things and then one genuine concern I ran into; take a look and let me know what you think?

CHANGELOG.md Outdated Show resolved Hide resolved
druid/examples/list_sources.rs Outdated Show resolved Hide resolved
druid/src/widget/list.rs Outdated Show resolved Hide resolved
druid/src/widget/list.rs Outdated Show resolved Hide resolved
@cmyr cmyr added S-waiting-on-author waits for changes from the submitter and removed S-needs-review waits for review labels Mar 11, 2021
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.

Thanks!

@cmyr cmyr merged commit d3f3550 into linebender:master Mar 12, 2021
derekdreery pushed a commit to derekdreery/druid that referenced this pull request Apr 6, 2021
* adding ListIter implementations and examples

* Changed example name, added modifying via buttons.

* handling changes of Key for OrdMap and HashMap

* Added lejero user, pull request detail

* changelog edit

* github spelling mistake

* change log overwrite

* formatting, adhering to standards

* adding changelog entry

* Added PR

* Added web examples entry for list_sources.rs

* Removed HashMap ListIter implementation.

* Update CHANGELOG.md

Co-authored-by: Colin Rofls <[email protected]>

* Update druid/examples/list_sources.rs

Co-authored-by: Colin Rofls <[email protected]>

* Removed Unnecessary Clone

* Removed HashMap Key and Value impl for ListIter and list_sources.rs example

Co-authored-by: Colin Rofls <[email protected]>
@xStrom xStrom removed the S-waiting-on-author waits for changes from the submitter label Jan 18, 2023
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