spike/5010-roettger-ScreenReaderResponsiveTransitionHeader #5047
Conversation
TimRoe
requested review from
timwright12,
narin,
alexandec,
theodur,
dumathane and
Sparowhawk
and removed request for
a team
| export function useIsScreanReaderEnabled(): boolean { | ||
| const [screanReaderEnabled, setScreanReaderEnabled] = useState(false) | ||
| export function useIsScreenReaderEnabled(withListener = false): boolean { | ||
| const [screenReaderEnabled, setScreenReaderEnabled] = useState(false) | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
A couple notes:
- A lot of updates in this file are just fixing the spelling error of "screan" to "screen"
- I added the withListener stuff making this potentially needlessly complicated to preserve the two existing use cases behaving exactly as before, I am not sure if it'd matter for those uses
There was a problem hiding this comment.
I'm not familiar with the details, but is it worth adding a todo for future cleanup or investigation?
There was a problem hiding this comment.
I have created an upkeep ticket #5057 to track cleaning it up at a later date.
…eTransitionHeader
…der' of https://github.com/department-of-veterans-affairs/va-mobile-app into spike/5010-roettger-ScreenReaderResponsiveTransitionHeader
| @@ -0,0 +1,8 @@ | |||
| import HooksInfo from '../../../../src/components/HooksInfo' | |||
|
|
|||
| export const exampleString = `const screenReaderEnabled = useIsScreenReaderEnabled()\n | |||
There was a problem hiding this comment.
Only changes were fixing the spelling typo.
…eTransitionHeader
…eTransitionHeader
|
I have some questions here: Did we need the listener? Why? And then did we indicate to QA the two pre-existing usecases for them to confirm functionality? Also @TimRoe in your example video, you never flip off Screenreader when in the page content. Can you play around turning off and on on the bottom of a page to make sure it handles that gracefully? Can you also please check if this will fix the issue reported in 4993? |
…eTransitionHeader
Yes because otherwise it would only be reflected based on the screen reader state (on/off) when entering a new screen, not be responsive when turned on within a screen. I did not since the function is functionally identical without the newly added argument to set it with an active listener, so for the purposes of the 2 other calling locations it did not change.
It does appear to work gracefully turning on/off at the bottom of the page: there's a slight shift in position on enable/disable with the subheader disappearing/reappearing, but otherwise behaves as I'd expect with starting focus on the header (back button or title on screens checked) and swiping into the details from there behaving as I think it did before for both iOS and Android (iOS scrolls the screen to top and hitting the first element, Android entering whatever the top element is at current scroll position).
I am unable to reproduce that on my iOS or Android devices with this branch, but I honestly couldn't tell you why it'd fix that. |
…eTransitionHeader
dumathane
deleted the
spike/5010-roettger-ScreenReaderResponsiveTransitionHeader
branch
Description of Change
Investigated whether or not we can have the transitioning header behavior be dynamic based on screen reader on/off status to avoid confusing edge cases for any veterans using the screen reader functionality.
This was possible and I have implemented the ideal scenario: when the screen reader is active or activated, the flat title in the screen disappears and is moved to the header where it will remain as long as the screen reader is active. This ensures alignment of what is read with what is highlighted, meeting expectations of screen title being in the header, and avoiding any duplicate reading or visual of the title.
The resolution also hides the header drop shadow when screen reader is on--mostly as the most convenient way to address not appear on content (Veteran's Crisis Line) that is flush against the header, but also because it's visual fluff that is unlikely to benefit a screen reader user.
Screenshots/Video
RPReplay_Final1678309981.MP4
Testing
Draft testing included:
Reviewer Validations
Attempt to implement AccessibilityInfo screen reader detection to make the transitioning header static when on
PR Checklist
Reviewer: Confirm the items below as you review