Gpii 4207 Updates to default values and some settings that were missing #824

sgithens · 2019-11-01T00:33:36Z

This contains sluething from testing results on the latest capture tool build.

@the-t-in-rtf Could you take a look at the JAWS settings I added? I did it all based on just toggling them in the JAWS UI and then looking at what changed in DEFAULT.JCF. Some of the bit mask fields obviously shouldn't be strings, but they seem to be just consistent repeats of the same number in each field so it seems like they should work for now.

- Adding temporary notes on some settings during investigation - Fixing some ranges and default values on settings that are straight forward.

gpii-bot · 2019-11-01T01:03:30Z

CI job passed: https://ci.gpii.net/job/universal-tests/1967/

…alue of a solution setting.

gpii-bot · 2019-11-04T01:34:42Z

CI job passed: https://ci.gpii.net/job/universal-tests/1969/

the-t-in-rtf · 2019-11-04T09:12:46Z

testData/solutions/win32.json5

+                            "type": "string",
+                            "default": "1|1|1|Access Key",
+                            "enum": [
+                                "0|0|0|Access Key",


Is this literally what's stored in the settings file?

Yup, it's the actual string, of three matching numbers and the label separated by pipes. I just copied them straight from the .ini file format after saving the dialog with different options. It's admittedly pretty wonky.

As long as we're clear with Gregg that we can only really handle them as enums for now, it doesn't matter how funny they get. They can use the table flipping ascii image as "false" for all we care.

@the-t-in-rtf I think it's ok. It is an enum, so this odd string isn't showing up in any of the user facing tools.

Yeah, if all of these are the literal values that are expected to be written to the INI file, then I'm fine. Just wanted to confirm as with the others.

the-t-in-rtf · 2019-11-04T09:13:17Z

testData/solutions/win32.json5

+                            "type": "string",
+                            "default": "1|1|1|Tutor",
+                            "enum": [
+                                "0|0|0|Tutor",


Same question here, is the literal value 0 here or the string you've put here?

Yup, same as above, this is the actual odd string value that shows up in the ini formatted settings.

the-t-in-rtf · 2019-11-04T09:14:44Z

testData/solutions/win32.json5

+                    //           Control Panel UI        GPII Capture
+                    // low:      100                     5000
+                    // high:     500                     1000  (yes it's inverse)
+                    // default:  300                     3000
                    "Acceleration": {
                        "schema": {
                            "title": "Mouse keys acceleration",
                            "description": "Acceleration of mouse keys",
                            "type": "number",


All the examples make it seem like this should be type integer instead of number?

That seems like it would be correct. Have updated it to integer

the-t-in-rtf · 2019-11-04T09:18:19Z

testData/solutions/win32.json5

@@ -6743,6 +6840,7 @@
                        "schema": {
                            "title": "Video audio description",
                            "description": "Hear descriptions of what's happening in videos",
+                            // GPII-4207 Test to see if this `default` is getting cast correctly


You are right to flag all of these with TODOs. Previously we have used an imprecise mix of truthy and falsy stuff to represent values like these, there was a lot of it previously in the JAWS settings as well. Until we come up with our own means of standardising these (for example, intra-application settings transforms), we should put the literal value the settings handler needs to read and write the correct value. We can get away with using booleans, which do cast to strings, but really need to be careful about representing ones and zeros in schemas as booleans.

gpii-bot · 2019-11-04T21:43:15Z

CI job passed: https://ci.gpii.net/job/universal-tests/1971/

gpii-bot · 2019-11-07T18:54:26Z

CI job passed: https://ci.gpii.net/job/universal-tests/1978/

gpii-bot · 2019-11-20T22:08:51Z

CI job passed: https://ci.gpii.net/job/universal-tests/2004/

amb26 · 2020-01-30T11:37:35Z

gpii/node_modules/solutionsRegistry/src/js/SolutionsUtils.js

+ * }
+ * ```
+ */
+gpii.universal.solutionsRegistry.findDefaultValue = function (schema) {


It's helpful, but it is not used by anything in the codebase - does it belong in the capture tool instead?
Also note that the one place where we do fetch setting defaults, we do not use this approach - https://github.com/GPII/universal/blob/master/gpii/node_modules/flowManager/src/PSPChannel.js#L139 - instead, we read the value from build/schemas/solution-schema-codex.json . What do you think, @cindyli , @the-t-in-rtf ?

I was confused as to where and why you'd use this, but then I looked at some of the deeper structures like the SPI settings here:

universal/testData/solutions/win32.json5

Line 6324 in 76ad0e6

// TODO: Discuss somehow simplifying the structure of SPI settings for easier UI input.

I guess my questions are, what do you expect to do with this potential "deep default" and why do you preserve the "type" but not anything else? If you're putting up controls or anything else, you don't have enough information to create controls. You don't know the range any longer, you don't know if the field is required, et cetera. If you're getting that info somewhere else, why not get the type from there as well?

Also, your code is focused on the value property, there are others, like this object that has RGB properties. How do you plan to handle those?

It seems to me like you need proper handling for nested properties, and this type of recursion should be possible in both javascript and handlebars templates. But maybe I just don't get what you're doing, and an example of what you're feeding into this and where you use it would help. Apologies if you've already covered that in the longer conversation.

I also am not sure this belongs in this particular part of the Solutions Registry. The use case would help confirm, but if it's truly about UI generation, it seems like this would be an early utility that would make up part of the "presentation registry" instead.

@amb26 Well, it's first use is the capture tool (and I've already temporarily duplicated it there), but it will all need to be used in the PPT, Morphic Vaults and other user facing applications build on top of universal. Quite a number of our UX specs at some point highlight what the default value of a particular setting is.

Also, regardless of the input, whether it's from win32.json or from solution-schema-codex.json, I believe we have the same problems. (and this impl I believe should still work if you pass in the corresponding property/schema either way. Here is a section from my locally generated solution-schema-codex.json:

"SlowKeysInterval": { "title": "Slow keys interval", "description": "Slow keys interval time in milliseconds", "properties": { "path": { "type": "string", "required": true }, "value": { "required": true, "type": "integer", "minimum": 0, "maximum": 20000, "default": 1000 } } }, "BounceKeysInterval": { "title": "Bounce keys interval", "description": "Bounce keys interval time in milliseconds", "type": "integer", "default": 0 }

Here, you can see that the default is at the top level for BounceKeysInterval, and then for SlowKeysInterval you need to drill down a level to get the default value.

@the-t-in-rtf An example of what I'm doing with the default value (regardless of whether it's in the top level of the items schema, or further down), is here in the capture tool where you can filter values on and off, depending if they match the default:

In the PPT, on the tables that show settings for each solution there is a column for the default.

Current I'm just preserving type because it's the amount of information I need to build these UI's (maybe not a great answer, but the truth). I just need the default, and it's type so I can compare them. I've haven't yet dealt with RGB values or anything, as the comments on the method allude to, I'm trying to knock out the largest number of settings at the moment that show up on these UI's with the simplest solution so far.

I'm ok with it being anywhere in universal, but it would be nice to have it in at least one of our most central repos... at the moment every one of our user facing projects still includes universal. Also, I can imagine a time soon where this default value information would be needed to create some time of data feed endpoint in universal.

I still don't understand why you can't just use the schema in its entirety. We should discuss in the meeting today.

the-t-in-rtf · 2020-01-30T13:36:48Z

gpii/node_modules/solutionsRegistry/src/js/SolutionsUtils.js

+    //         "default": "%SystemRoot%\\Web\\Wallpaper\\Windows\\img0.jpg"
+    //     }
+    // }
+    else if (schema.properties && schema.properties.value && schema.properties.value["default"] !== undefined) {


This seems like a great place to use fluid.get.

Good call, updated to use fluid.get

gpii-bot · 2020-02-03T22:54:01Z

CI job passed: https://ci.gpii.net/job/universal-tests/2125/

amb26 · 2020-05-05T14:17:20Z

Should be merged up to resolve some conflicts - does the capture work depend on this?

the-t-in-rtf · 2020-05-07T11:46:49Z

testData/solutions/win32.json5

                    "MaxSpeed": {
                        "schema": {
                            "title": "Mouse keys speed",
                            "description": "Speed of mouse keys",
                            "type": "number",
-                            "multipleOf": 10
+                            "multipleOf": 10,
+                            "default": 80


Awesome, very good use of the capture tool to fill these in.

the-t-in-rtf · 2020-05-07T11:47:23Z

testData/solutions/win32.json5

                                }
                            }
                        }
                    },
+                    // GPII-4207 This default looks correct... it probably isn't getting casted to


the-t-in-rtf · 2020-05-07T11:47:37Z

testData/solutions/win32.json5

+                                    // In the current Windows 10 UI this goes from 0.0 seconds
+                                    // to 20.0 seconds in somewhat random steps in a combo box:
+                                    // 0.0, 0.3, 0.5, 0.7, 1.0, 1.4, 2.0, 5.0, 10.0, 20.0
+                                    // TODO try manually setting a differnet value under the covers,


"different"

the-t-in-rtf · 2020-05-07T11:48:09Z

testData/solutions/win32.json5

@@ -4803,7 +4884,7 @@
        ]
    },
    "com.microsoft.windows.desktopBackground": {
-        "name": "Windows desktop background personalization",
+        "name": "Windows desktop background wallpaper",


Bless you for updating the wording on these.

the-t-in-rtf · 2020-05-07T11:50:51Z

testData/solutions/win32.json5

@@ -7117,6 +7212,7 @@
                            "title": "Keyboard as preferred input method",
                            "description": "Set the keyboard as the preferred input method",
                            "type": "boolean",
+                            // GPII-4207 This is coming out of the capture as 0 rather than false... TODO


I suspect there is some leftover sloppiness around truthy and falsy values that we'll need to pin down somehow, ideally when we have "intra-application settings" and can more easily have a user-facing true/false setting that is transformed to a 1/0 for a settings-handler specific setting.

the-t-in-rtf · 2020-05-07T11:56:20Z

I had a quick look and didn't see anything troubling, it's really only the upstream conflicts that need to be addressed from what I can see. Everything else is the same since February.

sgithens · 2020-05-08T02:07:27Z

Hi @the-t-in-rtf @kaspermarkus I went back through this again, and I think the parts of this that are absolutely required for the capture tool (1.3.5) have been addressed in other tickets. It would still be good for me to clean these up, and keep it tagged as 1.3.x, but we can remove the 1.3.5 label from this I believe.

sgithens added 2 commits October 31, 2019 14:10

GPII-4207 Adding some JAWS settings from the Basic Settings Dialog

e5ecc13

GPII-4207 Default Settings Investigation

de1a98a

- Adding temporary notes on some settings during investigation - Fixing some ranges and default values on settings that are straight forward.

sgithens added the Needs Work label Nov 1, 2019

GPII-4207 Adding utility function and tests for finding the default v…

2835aab

…alue of a solution setting.

the-t-in-rtf reviewed Nov 4, 2019

View reviewed changes

sgithens added 2 commits November 4, 2019 13:05

GPII-4207 Adding type information back in to findDefaultValue

f1ac07a

GPII-4207 Updating mousekeys acceleration to be an integer type.

556a578

GPII-4207 Disambiguating win32 desktop personalization names

b0416bd

GPII-4207 Removing some debug/investigating comments.

37687db

sgithens removed the Needs Work label Jan 10, 2020

amb26 reviewed Jan 30, 2020

View reviewed changes

the-t-in-rtf reviewed Jan 30, 2020

View reviewed changes

sgithens added 2 commits January 31, 2020 15:31

Merge remote-tracking branch 'GPII/master' into GPII-4207

fa7d3f3

GPII-4207 Cleaning up property access with fluid.get

3b26e2d

kaspermarkus added the Morphic 1.4 label Apr 13, 2020

kaspermarkus added the Morphic 1.3.5 label Apr 28, 2020

kaspermarkus added the Important label May 6, 2020

the-t-in-rtf reviewed May 7, 2020

View reviewed changes

testData/solutions/win32.json5

}

}

}

},

// GPII-4207 This default looks correct... it probably isn't getting casted to

Copy link

Member

the-t-in-rtf May 7, 2020

Choose a reason for hiding this comment

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

"cast"

the-t-in-rtf reviewed May 7, 2020

View reviewed changes

amb26 removed the Morphic 1.3.5 label Jun 5, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gpii 4207 Updates to default values and some settings that were missing #824

Gpii 4207 Updates to default values and some settings that were missing #824

sgithens commented Nov 1, 2019

gpii-bot commented Nov 1, 2019

gpii-bot commented Nov 4, 2019

the-t-in-rtf Nov 4, 2019

sgithens Nov 4, 2019

the-t-in-rtf Nov 5, 2019

sgithens Nov 20, 2019

the-t-in-rtf Nov 21, 2019

the-t-in-rtf Nov 4, 2019

sgithens Nov 4, 2019

the-t-in-rtf Nov 4, 2019

sgithens Nov 4, 2019

the-t-in-rtf Nov 4, 2019

gpii-bot commented Nov 4, 2019

gpii-bot commented Nov 7, 2019

gpii-bot commented Nov 20, 2019

amb26 Jan 30, 2020

the-t-in-rtf Jan 30, 2020

sgithens Feb 3, 2020

sgithens Feb 3, 2020

the-t-in-rtf Feb 4, 2020

the-t-in-rtf Jan 30, 2020

sgithens Feb 3, 2020

gpii-bot commented Feb 3, 2020

amb26 commented May 5, 2020

the-t-in-rtf May 7, 2020

the-t-in-rtf May 7, 2020

the-t-in-rtf May 7, 2020

the-t-in-rtf May 7, 2020

the-t-in-rtf May 7, 2020

the-t-in-rtf commented May 7, 2020 •

edited

Loading

sgithens commented May 8, 2020

Gpii 4207 Updates to default values and some settings that were missing #824

Are you sure you want to change the base?

Gpii 4207 Updates to default values and some settings that were missing #824

Conversation

sgithens commented Nov 1, 2019

gpii-bot commented Nov 1, 2019

gpii-bot commented Nov 4, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gpii-bot commented Nov 4, 2019

gpii-bot commented Nov 7, 2019

gpii-bot commented Nov 20, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gpii-bot commented Feb 3, 2020

amb26 commented May 5, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

the-t-in-rtf commented May 7, 2020 • edited Loading

sgithens commented May 8, 2020

the-t-in-rtf commented May 7, 2020 •

edited

Loading