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

System tray state machine #290

Open
wants to merge 49 commits into
base: main
Choose a base branch
from

Conversation

jesicasusanto
Copy link
Collaborator

@jesicasusanto jesicasusanto commented Jun 19, 2023

What kind of change does this PR introduce?

This PR resolves #224. This PR introduces a new feature to create a window that is always on top and allows customization of its styling. It updates the position of the window to be at the top right of the active window the user is interacting with. The contents of the window are dynamically updated to show different icons based on the active window and available recordings.

Summary

The motivation for making this change is to enhance the demo for the calculator sequence, making it smoother and providing indicator status for the recordings. This improvement aligns with the open issue #236 .

Checklist

  • My code follows the style guidelines of OpenAdapt
  • I have perfomed a self-review of my code
  • If applicable, I have added tests to prove my fix is functional/effective
  • I have linted my code locally prior to submission
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (e.g. README.md, requirements.txt)
  • New and existing unit tests pass locally with my changes

How can your code be run and tested?

@jesicasusanto jesicasusanto marked this pull request as draft June 19, 2023 11:35
@abrichr
Copy link
Member

abrichr commented Jun 19, 2023

Thanks for kicking this off @jesicasusanto ! Let's use .svg where possible 🙏

@jesicasusanto
Copy link
Collaborator Author

jesicasusanto commented Jun 23, 2023

Update
image

  • Modified appearance such that there is no close/minimize/maximize buttons and no title
  • Transparent except for the puterbot image that we will be updating
  • Fixed/always on top, e.g. https://github.com/JakubBlaha/KivyOnTop
  • update the contents of the window to show different icons based on what we know about the active window and avaliable recordings
  • update the position of the window to be at the top right of whatever active window the user is currently interacting with

@jesicasusanto jesicasusanto marked this pull request as ready for review June 26, 2023 12:24
)

def position_above_active_window(self, *args):
if sys.platform == "win32":
Copy link
Member

Choose a reason for hiding this comment

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

Thanks @jesicasusanto ! What do you think about re-using openadapt.window here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You're welcome @abrichr resolved in 6096a8f

self.window.size = (50, 50)
self.window.clearcolor = (255, 255, 255)
self.window.always_on_top = True
self.PROC = None
Copy link
Member

Choose a reason for hiding this comment

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

Since this isn't a constant, what do you think about naming it e.g. process? Even better, how about splitting it into replay_process and record_process?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

resolved in ecf0e65

active_window_properties["rectangle"].right,
)
except:
pass
Copy link
Member

Choose a reason for hiding this comment

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

As mentioned elsewhere I think it would be preferable to re-use openadapt.window here.

In general, however, swallowing exceptions like this is not recommended -- logging is preferable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

resolved in 6096a8f

@jesicasusanto
Copy link
Collaborator Author

Update :

@jesicasusanto
Copy link
Collaborator Author

To Do :

  • add callback whenever the window is moved

@abrichr abrichr mentioned this pull request Jul 2, 2023
7 tasks
@abrichr
Copy link
Member

abrichr commented Jul 2, 2023

@jesicasusanto @0dm I think we want to replace this and #300 as described in #300 (comment)

@abrichr
Copy link
Member

abrichr commented Jul 4, 2023

Let's replace the floating widget with a system tray icon

@abrichr abrichr changed the title Add Widgets for Windows System tray state machine Jul 7, 2023
@cr-gpt
Copy link

cr-gpt bot commented Jul 12, 2023

Seems you are using me but didn't get OPENAI_API_KEY seted in Variables/Secrets for this repo. you could follow readme for more information

1 similar comment
@cr-gpt
Copy link

cr-gpt bot commented Jul 12, 2023

Seems you are using me but didn't get OPENAI_API_KEY seted in Variables/Secrets for this repo. you could follow readme for more information

active_window_dict["state"].pop("data")
if reference_window_dict and "state" in reference_window_dict:
reference_window_dict["state"].pop("data")
if active_window_dict and "state" in active_window_dict :
Copy link
Member

Choose a reason for hiding this comment

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

Please format with black 🙏

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

addressed in 1027546

if first_window_id != last_window_id:
logger.warning(f"{first_window_id=} != {last_window_id=}")
ignore_window_ids.add(first_window_id)
ignore_window_ids.add(last_window_id)
logger.info(f"ignoring {first_window_title=} {last_window_title=}")
window_event_states = [
action_event.window_event.state
if action_event.window_event.state["window_id"] not in ignore_window_ids
if action_event.window_event.state is not None and action_event.window_event.state["window_id"] not in ignore_window_ids
Copy link
Member

Choose a reason for hiding this comment

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

I think we can remove is not None here

Copy link
Collaborator Author

@jesicasusanto jesicasusanto Jul 18, 2023

Choose a reason for hiding this comment

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

addressed in 49d3d32

@@ -8,7 +8,7 @@
import pickle


def get_active_window_state() -> dict:
def get_active_window_state(show_data) -> dict:
Copy link
Member

Choose a reason for hiding this comment

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

How about:

def get_active_window_state(metadata_only=False) -> dict:

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As addressed in bb45d40, after deciding to implement the tray state machine, we no longer require any type of flag on the get_active_window_state function. Previously, the flag was utilized to expedite obtaining the active window state, as it was necessary for positioning the widget above the active window.

pyproject.toml Outdated
@@ -43,6 +43,7 @@ ascii_magic = "2.3.0"
dictalchemy3 = "1.0.0"
fire = "0.4.0"
ipdb = "0.13.11"
kivy = "2.2.1"
Copy link
Member

Choose a reason for hiding this comment

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

Please remove if unused

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

addressed in 3a66d88

@abrichr
Copy link
Member

abrichr commented Jul 25, 2023

@jesicasusanto please fix merge conflicts 🙏

@jesicasusanto
Copy link
Collaborator Author

jesicasusanto commented Jul 25, 2023

addressed merge conflict e3afd5a @abrichr

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.

Explore MacOS/Windows widgets
2 participants