nvda icon indicating copy to clipboard operation
nvda copied to clipboard

Suggestion: make blockAction a submodule of utils instead of gui

Open XLTechie opened this issue 2 years ago • 2 comments

Is your feature request related to a problem? Please describe.

I think there may be other places that we can use blockAction in the future.

  • Config module, where some checks are currently done to prevent things from running in secure mode. I believe that some of these could be restructured to use blockAction.when()
  • In other parts of core where blocking conditions happen (logHandler? though it may be overkill there).
  • In add-ons that aren't otherwise using GUI, but may wish to block under some of the same circumstances.

For these, it is somewhat inappropriate to import part of gui (gui.blockAction).

I propose that it would make more sense to have this module in utils, where it can be picked up by anything that needs it, without implying gui involvement, or implying that blockAction should only be used in gui related operations.

Describe the solution you'd like

I would like to see gui.blockAction, become utils.blockAction.

Describe alternatives you've considered

Leave it like it is, and import from gui even in places that wouldn't otherwise.

Additional context

Additionally, I have some ideas for broadening the capabilities of blockAction in the future. For example, a blocker could be devised, or when() could be modified, that would return a callback instead of None, if a function is blocked. Or, a class could be returned, which could enable add-ons to use it to disable themselves on secure screens more easily by decorating the creation of GlobalPlugins. (Whether any of those ideas would ultimately be accepted, is not what I'm asking here.)

XLTechie avatar May 03 '22 07:05 XLTechie

CC @seanbudd since I believe you wrote it.

XLTechie avatar May 04 '22 02:05 XLTechie

Hi @XLTechie - focusing on the main request here.

As it stands, blockAction is intended for providing a user message when a user action cannot be completed. As it imports from gui and ui, any import of util.blockAction would do the same.

When developing blockAction, I felt like a next step could be to make it more generic, by making the ui.message being optional. I figured the API needed to rest for a bit first, and this should be handled in a separate piece of work if necessary.

Doing this could take out the ui dependency where appropriate. The gui modalMessageBox context could be an injected dependency instead of hosted in util.

For example, this diff:

-def when(*contexts: Context):
+def when(*contexts: Context, message: bool = True):
 	"""Returns a function wrapper.
 	A function decorated with `when` will exit early if any supplied context in `contexts` is active.
 	The first supplied context to block will be reported as a message.
@@ -55,7 +55,9 @@ def when(*contexts: Context):
 		def funcWrapper(*args, **kwargs):
 			for context in contexts:
 				if context.blockActionIf():
-					queueHandler.queueFunction(queueHandler.eventQueue, ui.message, context.translatedMessage)
+					if message:
+						import ui
+						queueHandler.queueFunction(queueHandler.eventQueue, ui.message, context.translatedMessage)
 					return
 			return func(*args, **kwargs)
 		return funcWrapper

Alternatively, a separate decorator intended for this functionality could be created in util.

seanbudd avatar May 04 '22 03:05 seanbudd