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

Add ReadingBlock for MyChallengeAccordinBox content #440

Closed
zepumph opened this issue Mar 2, 2022 · 13 comments
Closed

Add ReadingBlock for MyChallengeAccordinBox content #440

zepumph opened this issue Mar 2, 2022 · 13 comments

Comments

@zepumph
Copy link
Member

zepumph commented Mar 2, 2022

Related to #381

https://docs.google.com/document/d/1offJbONUg8y5Aa9kHewC9iTDtNISzna4giGt7WrpXCk/edit#heading=h.tmpeoajac3hg
#363

@zepumph zepumph self-assigned this Mar 2, 2022
@zepumph zepumph changed the title Add readingblock for MyChallengeAccordinBox content Add ReadingBlock for MyChallengeAccordinBox content Mar 2, 2022
@zepumph
Copy link
Member Author

zepumph commented Mar 2, 2022

This should be fully implemented. @terracoda please review.

@zepumph
Copy link
Member Author

zepumph commented Mar 2, 2022

  • One interesting item here, is if you press a numberPicker button until it reaches the max, then that increment/decrement because pickable:false, and if you click again the ReadingBlock get's pressed. I'm not bothered by that, but it is a bit unexpected from a design perspective.

@zepumph
Copy link
Member Author

zepumph commented Mar 2, 2022

  • I just noticed that the reading block is before the number pickers in the focus order. That was what it was by default, let me know if we need to change it.

@terracoda
Copy link
Contributor

Yes, please move reading block

  • It should come after the Right Value number spinner in the Tab Order

And keep it as an all-input reading block.

New strings for the reading block:

  • Current challenge {{1}} to {{2}}. (same as PDOM context response)
  • Hint Response (when hints are checked in Preferences):
    • Change challenge, and move hands.

@terracoda
Copy link
Contributor

Re #440 (comment)

Yes, I think controls inside reading blocks will have to work like that. That is how it works in GFLB as well. So, let's keep it like that and be aware of it.

@terracoda
Copy link
Contributor

And the number spinners won't have a hint, see related issue #441

@zepumph
Copy link
Member Author

zepumph commented Mar 17, 2022

Ok The reading block should be done. I updated the focus order, removed the NumberPicker hint responses, and changed the reading block hint response.

@terracoda
Copy link
Contributor

Sorry @zepumph, I did not explain the Tab Order explicitly.

The My Challenge button should stay where it was in the Tab Order, and the reading block should only be in one spot. On first tab through I seem to be able to get tot eh reading block twice before and after the number spinners.

When the accordion panel is open, it should have 3 Tab stops after the actual My Challenge heading/button, one for each number spinner and 1 for the reading block. When the accordion panel is closed, it should have 1 tab stop, the My Challenge heading/button.

Tabbing Order from top to bottom when Accordion is open:

  • Range: 1 to 10
  • My Challenge
  • Left Value
  • Right Value
  • Reading Block (Current Challenge {{1}} to {{2}}.)
  • Ratio Lock
  • Reset All

Tabbing Order from bottom to top when Accordion is open:

  • Reset All
  • Ratio Lock
  • Reading Block (Current Challenge {{1}} to {{2}}.)
  • Right Value
  • Left Value
  • My Challenge
  • Range: 0 to 10

In addition, the hint response content is correct, but the actual Reading Block content needs to be changed to "Current Challenge {{1}} to {{2}}".

This string is shared with the PDOM, so updating removes the need for a custom string.

@zepumph
Copy link
Member Author

zepumph commented Mar 18, 2022

I think #446 and this issue are related. I am getting a strange assertion about PDOM order/DAG, which is also why we were getting duplicate ReadingBlock highlights. I'll take a look.

@zepumph
Copy link
Member Author

zepumph commented Mar 18, 2022

Ok, sorry, the focus order is buggy, but will be handled in phetsims/scenery#1385. Sorry for the confusion, I thought it was working but didn't test this case enough.

@zepumph
Copy link
Member Author

zepumph commented Mar 18, 2022

@jessegreenberg and I discussed the focus order issue, and solved it in this commit. I'll update the context response next.

@zepumph
Copy link
Member Author

zepumph commented Mar 18, 2022

See #381 for review.

@terracoda
Copy link
Contributor

The reading block works great with pointer and focus inputs. Nice work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants