From c0d475a5e247d97f2926df8c98846fd57b3c4fc9 Mon Sep 17 00:00:00 2001 From: KshitizSareen Date: Thu, 9 Feb 2023 12:15:04 -0800 Subject: [PATCH 1/6] PasswordInput: Fix overlap for long passwords and show/hide button Long passwords and show/hide buttons overlap with each other. I fixed this problem by doing the following: - I changed the style of Password Input to take up as much space as needed. I did this by setting a flex of 1 to the Password Input. - I changed the positioning of the show/hide button from absolute to relative. - I ensured horizontal alignment by setting the flex-direction of the parent view to 'row'. - To fix the problem of the changing width of the text input, when the show/hide text changes, I replaced the show/hide text with a show/icon. The show/hide icon is an eye-open/eye-closed icon. This icon always has a fixed width and does not affect the text input. - I removed hide text from messages_en.json since it is not in the app. I tested this on Android and IOS devices with different screen sizes, and it works fine on both platforms. Fixes: #5614 --- src/common/PasswordInput.js | 26 +++++++++++++++++--------- static/translations/messages_en.json | 1 - 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/common/PasswordInput.js b/src/common/PasswordInput.js index 5c21ec21bb6..8d90e84b2ef 100644 --- a/src/common/PasswordInput.js +++ b/src/common/PasswordInput.js @@ -3,24 +3,26 @@ import React, { useState, useCallback } from 'react'; import type { Node } from 'react'; import { View } from 'react-native'; +import { Icon } from './Icons'; import Input from './Input'; import type { Props as InputProps } from './Input'; import { BRAND_COLOR, createStyleSheet } from '../styles'; -import ZulipTextIntl from './ZulipTextIntl'; import Touchable from './Touchable'; const styles = createStyleSheet({ showPasswordButton: { - position: 'absolute', - right: 0, - top: 0, - bottom: 0, justifyContent: 'center', }, - showPasswordButtonText: { + showPasswordButtonIcon: { margin: 8, color: BRAND_COLOR, }, + passwordTextInput: { + flex: 1, + }, + passwordInputView: { + flexDirection: 'row', + }, }); // Prettier wants a ", >" here, which is silly. @@ -44,10 +46,16 @@ export default function PasswordInput(props: Props): Node { }, []); return ( - - + + - + ); diff --git a/static/translations/messages_en.json b/static/translations/messages_en.json index cee337d8a1f..b153479ce93 100644 --- a/static/translations/messages_en.json +++ b/static/translations/messages_en.json @@ -279,7 +279,6 @@ "Stream settings": "Stream settings", "Failed to show stream settings": "Failed to show stream settings", "show": "show", - "hide": "hide", "Debug": "Debug", "Administrators": "Administrators", "Normal users": "Normal users", From 4b788752441150d83dda8fae75e785922b3cb02d Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 10 Feb 2023 11:28:12 -0800 Subject: [PATCH 2/6] PasswordInput: Add 8px margin between password input and show/hide button The touchable-feedback area was right up against the right edge of the password input. Give some margin in between, for some breathing room. --- src/common/PasswordInput.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/common/PasswordInput.js b/src/common/PasswordInput.js index 8d90e84b2ef..70f7fc2ba8a 100644 --- a/src/common/PasswordInput.js +++ b/src/common/PasswordInput.js @@ -12,6 +12,7 @@ import Touchable from './Touchable'; const styles = createStyleSheet({ showPasswordButton: { justifyContent: 'center', + marginLeft: 8, }, showPasswordButtonIcon: { margin: 8, From 3682bc31355319d1427c35373e53568b4c5c1d87 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 10 Feb 2023 11:08:56 -0800 Subject: [PATCH 3/6] PasswordInput: Don't let show/hide button stretch password input's height The default value for `alignItems` is 'stretch': https://reactnative.dev/docs/0.68/flexbox#align-items --- src/common/PasswordInput.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/PasswordInput.js b/src/common/PasswordInput.js index 70f7fc2ba8a..6a80df7221c 100644 --- a/src/common/PasswordInput.js +++ b/src/common/PasswordInput.js @@ -11,7 +11,6 @@ import Touchable from './Touchable'; const styles = createStyleSheet({ showPasswordButton: { - justifyContent: 'center', marginLeft: 8, }, showPasswordButtonIcon: { @@ -23,6 +22,7 @@ const styles = createStyleSheet({ }, passwordInputView: { flexDirection: 'row', + alignItems: 'center', }, }); From 322b330a0f63f7b2d503c4117188584945633002 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 10 Feb 2023 11:33:44 -0800 Subject: [PATCH 4/6] PasswordInput: Give show/hide button a 48px touch target size --- src/common/PasswordInput.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/common/PasswordInput.js b/src/common/PasswordInput.js index 6a80df7221c..e868ae46aec 100644 --- a/src/common/PasswordInput.js +++ b/src/common/PasswordInput.js @@ -14,7 +14,9 @@ const styles = createStyleSheet({ marginLeft: 8, }, showPasswordButtonIcon: { - margin: 8, + // 24 (icon size) + 12 + 12 = 48px min touch target: + // https://material.io/design/usability/accessibility.html#layout-and-typography + margin: 12, color: BRAND_COLOR, }, passwordTextInput: { From 3f3324fef2d424293e2682aa13b4039d5e9955d6 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 10 Feb 2023 10:56:58 -0800 Subject: [PATCH 5/6] PasswordInput: Use Pressable for show/hide icon button This lets us have a more compact layout (e.g., the left edge of the icon is just 8px away from the right edge of the password input, instead of that 8px plus 12px of padding in the Touchable) while maintaining the 48px touch-target size with the help of `hitSlop`. --- src/common/PasswordInput.js | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/src/common/PasswordInput.js b/src/common/PasswordInput.js index e868ae46aec..d06c3004ba0 100644 --- a/src/common/PasswordInput.js +++ b/src/common/PasswordInput.js @@ -1,24 +1,17 @@ /* @flow strict-local */ import React, { useState, useCallback } from 'react'; import type { Node } from 'react'; -import { View } from 'react-native'; +import { View, Pressable } from 'react-native'; import { Icon } from './Icons'; import Input from './Input'; import type { Props as InputProps } from './Input'; -import { BRAND_COLOR, createStyleSheet } from '../styles'; -import Touchable from './Touchable'; +import { BRAND_COLOR, HIGHLIGHT_COLOR, createStyleSheet } from '../styles'; const styles = createStyleSheet({ showPasswordButton: { marginLeft: 8, }, - showPasswordButtonIcon: { - // 24 (icon size) + 12 + 12 = 48px min touch target: - // https://material.io/design/usability/accessibility.html#layout-and-typography - margin: 12, - color: BRAND_COLOR, - }, passwordTextInput: { flex: 1, }, @@ -57,9 +50,21 @@ export default function PasswordInput(props: Props): Node { autoCapitalize="none" style={styles.passwordTextInput} /> - - - + + {({ pressed }) => ( + + )} + ); } From 9c473e1f33c2c2a441502ac62ce72d4a4cdd1c11 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 10 Feb 2023 11:48:20 -0800 Subject: [PATCH 6/6] PasswordInput: Add accessibility properties to show/hide button Now, after changing the show/hide button to be an icon instead of text, we still present user-facing text to screen reader software. Tested on iOS with VoiceOver, and implemented with help from this article: https://incl.ca/show-hide-password-accessibility-and-password-hints-tutorial/ --- src/common/PasswordInput.js | 6 ++++++ static/translations/messages_en.json | 1 + 2 files changed, 7 insertions(+) diff --git a/src/common/PasswordInput.js b/src/common/PasswordInput.js index d06c3004ba0..fa662f15116 100644 --- a/src/common/PasswordInput.js +++ b/src/common/PasswordInput.js @@ -7,6 +7,7 @@ import { Icon } from './Icons'; import Input from './Input'; import type { Props as InputProps } from './Input'; import { BRAND_COLOR, HIGHLIGHT_COLOR, createStyleSheet } from '../styles'; +import { TranslationContext } from '../boot/TranslationProvider'; const styles = createStyleSheet({ showPasswordButton: { @@ -35,6 +36,8 @@ type Props = $ReadOnly<$Diff(true); const handleShow = useCallback(() => { @@ -56,6 +59,9 @@ export default function PasswordInput(props: Props): Node { hitSlop={12} style={styles.showPasswordButton} onPress={handleShow} + accessibilityLabel={_('Show password')} + accessibilityRole="switch" + accessibilityState={{ checked: !isHidden }} > {({ pressed }) => (