-
Notifications
You must be signed in to change notification settings - Fork 3
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
Update Details read-me button content #78
Comments
Ok, the tables in the design document is ready, I have copied images here as a record of the design. Please do no include the rows with the grey background. They are just there as edge cases for now. @jessegreenberg, I tried to make it easier for you to break up the phrases & parameters by only having unique phrases in each cell. The ditto marks mean the string from the cell above applies. And "n/a" means there is no described property for this shape. I hope it is all clear. Note that in the second table, the only difference in the trapezoids' parallel-ness phrase is punctuation - a period for trapezoid and a comma for isosceles trapezoid. Table: Details & PDOM Summary when Named Shape IS a ParallelogramExamples:
Table: Details & PDOM Summary when Named Shape IS NOT a ParallelogramExamples:
|
The above definitions were based on minimal definitions, so I need to iterate to include a full definition of paralell-ness, side length and corners for each and any shape. |
Ok, I think I have a solid draft of phrases and defined parameters. I would like to discuss this latest version with @BLFiedler before assigning over to @jessegreenberg Shape Properties: Phrases & Parameters for Details & PDOM Summary |
I do see the need for specific information when the resolution of the side length or angle differences is not enough. Especially with the Trapezoid case, where the 1 set of parallel lines is not quite obvious from the description there. Definitely worth a deeper discussion, but it's looking really good! |
@jessegreenberg, in the design doc I have updated the tables posted in previous comments. I am still working on the rules for non-parallelogram shapes. Everything is ready for parallelogram shapes. Everything, that is, except how best to determine the starting vertex and side. The relevant sections in the design doc start here in Rules for Shape Properties (Dynamic State Descriptions) Will assign to you when rules for non-parallelograms are documented. |
How to determine the Corner
How to determine the Side
|
@BLFiedler suggested a different structure for the ordering logic: Within a sentence or between sentences, when ordering sets of corners or sides:
*When ordering within the above sets:
Assumptions:
Note that the * emphasizes that the ordering within any "equal" or "parallel" statement is subject to the bottom/left rules. |
@jessegreenberg, the design doc has the latest iteration of the above table screen shots. Here are screen shots of Detail 1 and Details 1 & 2 just for a few parallelogram. There should not be any changes to the phrasing of the 3 main Details of the Shape Properties, but I am still trying to add some details that might be helpful to you for actual phrases, so i don't want to copy that big table in yet. That said, I don't think the phrase details should hold up implementation. Just let me know if you have any questions. Detail 1: General description of shape’s corners and sides.
Detail 2: Shape-specific corner descriptions.
Detail 3: Shape-specific side descriptions.
|
The ordering logic is in #78 (comment) |
The above commits complete details 2, ready to start details 3. |
OK the "details" button is done and ready for review. There is still some cleanup to do but the Voicing is working. The only thing I had a little trouble with was ordering the side descriptions according to the priority described in the doc (parallel >equal >vertical >horizontal order). I believe it is correct but there were a few examples in the design doc that did not match the output currently in the sim. @terracoda this is ready for you to review. This change added ~1100 lines of code though, I am sure there are bugs in there! Let me know what you find. |
In 4/5/22 meeting, we discussed that I didn't get the ordering of the sides quite right. It should always be relative to the first vertex of the side (instead of determining which side is lowest). Back to me to make this change. |
The description implementation is really good, though I know you are still working on ordering of corner and side names. One thing I noticed while listening is that there is very little space between Detail 2 and Detail 3. Can you make sure there is a period there (a full stop) and a space. This is quite pronounced in the Parallelogram descriptions. Is there any way to view the Voiced text on the screen, like in show model values, or something similar. Text on the screen might be a future feature, so it would be nice to have regardless :-). I'll make an issue, so you can work on it separately. |
@jessegreenberg, I'll create a separate issue to implement the dynamic scene summary since Voicing is the priority. |
Great!
You can run the sim with query param
I hear this too, but when I use But I hear almost no space between sentences 2 and 3. |
OK, the ordering of sides has been adjusted to match the prioritization in the design doc. @terracoda back to you to review and to review my comment in #78 (comment). |
This ordering prioritization seems to be working well. @jessegreenberg, it would be great to see the Voicing on the screen, so I can identify any punctuation or phrasing issues that affect the quality of pronunciation in web speech. In some places, I am not hearing a pause when I think there should be a comma or period. This makes things sound smooshed together. |
I'll open a separate issue about making the Voiced text available...maybe @zepumph already has a solution for that in the console? |
In RaP, I can use &printVoicingResponses. I'll try that for Quad. |
I'll open an issue to see if this query parameter is easy to add to Quad. |
I thought we were missing a case, but we are not :-) |
The content for the dynamic part of the scene summary and the Details button in Voicing are currently exactly the same, and we hope to keep it that way.
Still updating the table in the design doc.
This issue is also related to checklist issue #74.
The text was updated successfully, but these errors were encountered: