Make gui.MainFrame.popupSettingsDialog part of the public API

[seanbudd]

Link to issue number:

Fixes #15120

Summary of the issue:

If add-ons want to provide a gesture or similar to open their config dialogs, they currently have two choices:

• Call gui.mainFrame._popupSettingsDialog, which is a private method and could thus change signature or mechanism at any moment without breakage warning. Several add-ons currently live on the edge by using this approach, including one of mine.
• Replicate the internals of gui.MainFrame._popupSettingsDialog to achieve the same thing, which involves some imports and careful management of parents and exceptions.

Description of user facing changes

None

Description of development approach

Mark API as public

Update AddonStore command to use the function

Testing strategy:

Tested opening and closing add-on store

Known issues with pull request:

None

Change log entries:

N/A

Code Review Checklist:

• Pull Request description:
  • description is up to date
  • change log entries
• Testing:
  • Unit tests
  • System (end to end) tests
  • Manual testing
• API is compatible with existing add-ons.
• Documentation:
  • User Documentation
  • Developer / Technical Documentation
  • Context sensitive help for GUI changes
• UX of all users considered:
  • Speech
  • Braille
  • Low Vision
  • Different web browsers
  • Localization in other languages / culture than English
• Security precautions taken.

[XLTechie]

Thanks @seanbudd! I suggest a note in the changes for developers that this has become public.

[AppVeyorBot]

• PASS: Translation comments check.
• PASS: Unit tests.
• FAIL: Lint check. See test results for more information.
• PASS: System tests (tags: installer NVDA).
• Build (for testing PR): https://ci.appveyor.com/api/buildjobs/uxahklcmjg5a9f2r/artifacts/output/nvda_snapshot_pr15121-28650,3aca9861.exe
• CI timing (mins):
  INIT 0.0,
  INSTALL_START 0.8,
  INSTALL_END 0.9,
  BUILD_START 0.0,
  BUILD_END 24.4,
  TESTSETUP_START 0.0,
  TESTSETUP_END 0.3,
  TEST_START 0.0,
  TEST_END 21.9,
  FINISH_END 0.1

See test results for failed build of commit 3aca986134

[seanbudd]

@michaelDCurran - re-requesting review as the lint fix was non-trivial

[XLTechie]

@seanbudd sorry that turned into such a PITA.

[jmdaweb]

Hi,
Since the add-ons mailing list is not the right place to discuss these things (as stated by the moderator), I suggest adding an alias for this method to keep compatibility until NVDA 2024.1. Although the function was not public, many add-ons make use of it and will break on 2023.2. This is the relevant thread: https://nvda-addons.groups.io/g/nvda-addons/topic/100136959
Regards.

[nvdaes]

Hello: Seems that this PR has been merged even wrongly. When I see the log, this appears:

commit a8b79c7
Author: Sean Budd [email protected]
Date: Fri Jul 14 17:13:13 2023 +1000

Make gui.MainFrame.popupSettingsDialog part of the public API (#15121)

Fixes #15096

Summary of the issue:
When an active profile has the synthesizer set to silence, and a user switches profiles to a synthesizer which fails to load, NVDA fails back to the silence synthesizer.

Also, as mentioned in issue #15141 , this breaks compatibility with add-ons. Additionally, in issue #15142 , vision provided are broken.
I think it maybe better to revert this and, since it's not urgent, and NVDA 2023.2 is almost complete, and we need to test relevant things like support for localizations in the store, I think this maybe reverted and set the milestone to 2024.1.

Author: Sean Budd [email protected]
Date: Fri Jul 14 17:13:13 2023 +1000

Make gui.MainFrame.popupSettingsDialog part of the public API (#15121)

Fixes #15096

Summary of the issue:
When an active profile has the synthesizer set to silence, and a user switches profiles to a synthesizer which fails to load, NVDA fails back to the silence synthesizer.

[XLTechie]

@seanbudd since this appears to have broken all sorts of things, mainly because of the change in import structure from the linting I think, but secondarily because no alias means that rather a few add-ons are immediately broken: Even though this was my idea, and I appreciate how fast you acted on it, it may be best to revert it and approach it in a more circumspect way. I suggest reverting this. Then, in the next PR on this point, changing *only* the single function, with appropriate deprecation alias according to deprecation.md. Even though it isn't a public method, enough people wrongly depended on it, that it has created more of an outcry than I expected. Although originally I had been thinking we would do an alias for it. Anyway, if we do an alias for now, the massive number of lints required that caused the vision and braille breaks can be handled more deliberately. Since this is indirectly my fault, I can do an initial pr to that effect if you're up for it, and incidentally the add-ons can have time to adapt. cc @michaelDCurran

[ABuffEr]

I think it maybe better to revert this and, since it's not urgent, and NVDA 2023.2 is almost complete, and we need to test relevant things like support for localizations in the store, I think this maybe reverted and set the milestone to 2024.1.

I completely agree with this proposal.