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

Refactor toolbar into an ipywidgets subclass #1664

Merged
merged 6 commits into from
Aug 17, 2023

Conversation

bengalin
Copy link
Collaborator

Created a new dataclass, Toolbar.Item, that defines an entry in the Toolbar.

Once layer_manager_gui() is widgetized, we can move the new Toolbar widget into map_widgets.py. The code remaining in toolbar.py will become simpler as the view and control separate.

Future work can also include implementing plotly_toolbar the same way.

Note that I uncommented the implementations for the help and gee tool items. If they are not needed, we can remove them (I think it is better to remove than to keep commented out code)

@naschmitz naschmitz added the refactor Used for code refactors label Aug 15, 2023
@bengalin bengalin requested a review from giswqs August 15, 2023 18:39
@giswqs
Copy link
Member

giswqs commented Aug 16, 2023

@bengalin Thank you for working on this. It seems there are some merge conflicts after #1663 has been merged. Can you fix the conflict? Thanks.

@bengalin
Copy link
Collaborator Author

Hmm, I somehow managed to close the pull request. I am still learning how to work with GitHub. Will see if I can recover this PR post merge or if I need to create a new one.

@giswqs
Copy link
Member

giswqs commented Aug 16, 2023

Is there a "Reopen pull request" on this page from your account?

image

@bengalin
Copy link
Collaborator Author

There is, but it is disabled and says "There are no new commits on the bengalin:widgets branch". So I guess the commits I have locally are no longer on my remote branch. And locally, git log shows my origin/master, origin/HEAD, and master to be at a July 30 commit:

$ git log
commit a896c0c0c23effac6a9b2744989af2e562609158 (HEAD -> widgets, origin/widgets)
Author: Ben Galin <[email protected]>
Date:   Tue Aug 15 19:50:05 2023 +0000

    Use `ee_initialize=False` in unit tests

commit bdb3d3e7f25041776ca2ba37f0c7f198f8878a7e
Author: Ben Galin <[email protected]>
Date:   Tue Aug 15 19:40:05 2023 +0000

    Fixed typo

commit 0de8a7a53596d77e4ef505bb8eeaee6aa19e1623
Author: Ben Galin <[email protected]>
Date:   Tue Aug 15 01:10:40 2023 +0000

    Remove unnecessary import

commit e500ec630b67a06aaa801f1b02c651bfdc190a22
Author: Ben Galin <[email protected]>
Date:   Mon Aug 14 18:17:45 2023 +0000

    Refactor toolbar into an ipywidgets subclass

commit 95a4c3bfa9be3ce9456fa80133aaf7dc92defbbf (origin/master, origin/HEAD, master)
Author: Qiusheng Wu <[email protected]>
Date:   Sun Jul 30 21:35:53 2023 -0400

    Bump version: 0.24.4 → 0.25.0

commit 189a833140bf17227ab14594209be7c0beed7ef6
Author: Qiusheng Wu <[email protected]>
Date:   Sun Jul 30 21:35:44 2023 -0400

    Prep for v0.25.0

@giswqs
Copy link
Member

giswqs commented Aug 16, 2023

You might have accidentially overwrite your local branch with the remote master branch. I can no longer see your commits

@bengalin bengalin reopened this Aug 16, 2023
@bengalin
Copy link
Collaborator Author

Okay, I managed to fetch and merge locally, and then after some magic the reopen PR button was active again. So I think we are good?

@giswqs
Copy link
Member

giswqs commented Aug 17, 2023

This is really well done! I tested it and everything works fine. I really like the refactoring. Much more elegant than my previous version. Learned some new tricks from your code. Thank you very much for your contribution!

@giswqs giswqs merged commit 423f499 into gee-community:master Aug 17, 2023
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Used for code refactors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants