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

GUI: Add a tab for backend-specific options #2591

Closed
wants to merge 2 commits into from

Conversation

@ccawley2011
Copy link
Member

@ccawley2011 ccawley2011 commented Nov 2, 2020

As a demonstration, this is implemented in the Android backend, which replaces the use of the kFeatureTouchpadMode and kFeatureOnScreenControl features.

@antoniou79
Copy link
Contributor

@antoniou79 antoniou79 commented Nov 2, 2020

It's an interesting change. Is this done to provide better isolation for such features ? (These features already appeared in a "Control" tab separately, but the code behind that was not a clear that those were backend specific?)

My main concern is whether a regression would be introduced by the example case for Android -- since in a previous fix (e25ac0b#diff-d112ba6d5371b6b878f86ddd82035bd646f53e85e0a2cc5164233b36f009c11d) I thought it would be better to explicitly set values for un-configured settings so that they would have actually their "keys" registered with ConfMan and hasKey() (if used by any external method, before retrieving the key value) would return "true" properly.

@ccawley2011
Copy link
Member Author

@ccawley2011 ccawley2011 commented Nov 2, 2020

It's an interesting change. Is this done to provide better isolation for such features ? (These features already appeared in a "Control" tab separately, but the code behind that was not a clear that those were backend specific?)

Yes, that was part of the reason for doing this. The other part was to allow the custom dialogs in some of the other ports to be replaced with something that's integrated into the main GUI.

My main concern is whether a regression would be introduced by the example case for Android -- since in a previous fix (e25ac0b#diff-d112ba6d5371b6b878f86ddd82035bd646f53e85e0a2cc5164233b36f009c11d) I thought it would be better to explicitly set values for un-configured settings so that they would have actually their "keys" registered with ConfMan and hasKey() (if used by any external method, before retrieving the key value) would return "true" properly.

That shouldn't be an issue in this case, since registerDefaultSettings is called much earlier in initialization than initBackend is, which seems to fix the problem.

@antoniou79
Copy link
Contributor

@antoniou79 antoniou79 commented Nov 3, 2020

Then this PR looks good to me.

@criezy
Copy link
Member

@criezy criezy commented Nov 4, 2020

This seems like a good idea to me, and one that could potentially be useful of the iOS backend.
And from a quick glance the implementation in this PR looks OK.

@bluegr
Copy link
Member

@bluegr bluegr commented Nov 13, 2020

I also love these changes :) Nice work!

@lotharsm
Copy link
Member

@lotharsm lotharsm commented Nov 15, 2020

Can we merge this soon so I can rebase #2621?

@lotharsm
Copy link
Member

@lotharsm lotharsm commented Nov 21, 2020

If there are no objections, I'll merge it this weekend, so we can progress with the other upcoming GUI changes.

@criezy
Copy link
Member

@criezy criezy commented Nov 22, 2020

I merged it manually to fix conflicts.

@criezy criezy closed this Nov 22, 2020
@ccawley2011 ccawley2011 deleted the ccawley2011:backend-options branch Nov 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.