Mouse Show/Hide feature #56
Conversation
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Hi @MikeGillotti, many thanks for your PR!
I just left some comments and, if you apply requested changes, I will accept your PR ;-)
manim_slides/present.py
Outdated
| @@ -562,7 +562,15 @@ def __init__( | |||
| self.thread.start() | |||
|
|
|||
| def keyPressEvent(self, event): | |||
| self.config = Config() | |||
There was a problem hiding this comment.
| self.config = Config() |
Here, you define a default config for every key press. That is quite inefficient and does not use the config that was passed to self.thread. See above
There was a problem hiding this comment.
Edit: can you modify Display.__init__ to take config: Config = Config() instead of just config? Then, also add config: Config = Config() to App.__init__ arguments, and modify the line self.thread = Display(*args, **kwargs) to be self.thread = Display(*args, config=config, **kwargs).
manim_slides/present.py
Outdated
| if self.hide_mouse == False and self.config.HIDE_MOUSE.match(key): | ||
| self.setCursor(Qt.BlankCursor) | ||
| self.hide_mouse = True | ||
| elif self.hide_mouse == True and self.config.HIDE_MOUSE.match(key): | ||
| self.setCursor(Qt.ArrowCursor) | ||
| self.hide_mouse = False | ||
| # We send key to be handled by video display | ||
| self.send_key_signal.emit(key) | ||
| event.accept() |
There was a problem hiding this comment.
| if self.hide_mouse == False and self.config.HIDE_MOUSE.match(key): | |
| self.setCursor(Qt.BlankCursor) | |
| self.hide_mouse = True | |
| elif self.hide_mouse == True and self.config.HIDE_MOUSE.match(key): | |
| self.setCursor(Qt.ArrowCursor) | |
| self.hide_mouse = False | |
| # We send key to be handled by video display | |
| self.send_key_signal.emit(key) | |
| event.accept() | |
| if self.config.HIDE_MOUSE.match(key): | |
| if self.hide_mouse: | |
| self.setCursor(Qt.ArrowCursor) | |
| self.hide_mouse = False | |
| else: | |
| self.setCursor(Qt.BlankCursor) | |
| self.hide_mouse = True | |
| else: | |
| # We send key to be handled by video display | |
| self.send_key_signal.emit(key) | |
| event.accept() |
There is no need to send the key to the thread if we already handled it ;-)
Applied requested changes for PR
for more information, see https://pre-commit.ci
|
Thank you for your feedback. I applied the requested changes. |
|
Look perfect to me, thank you @MikeGillotti! Merging this now |
Closes #51