feat(a11y): toasts to native dialogs - #6514
Conversation
GitHub Actions
Error
GitHub Actions
Inactive
GitHub Actions
Error
GitHub Actions
Inactive
GitHub Actions
Error
|
Android Build Available Rocket.Chat Experimental 4.63.0.93296 Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNTiOnozLfxMgmPjgnj1O7jL-2xTyDZifnMXwfutasqqXzkyMmeenSE_1ChoTYwCART5oBeP-nHwOUPwlEgl |
|
iOS Build Available Rocket.Chat Experimental 4.63.0.96969 |
GitHub Actions
Inactive
GitHub Actions
Error
GitHub Actions
Error
GitHub Actions
Inactive
GitHub Actions
Error
|
Android Build Available Rocket.Chat Experimental 4.64.0.93301 Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNRbb-ieTauA4kSJRQp1PFv85dnHiJurE1A6cIiWM49BsPjK3UAge3O35UKAJ64-zIzTsgxNj0HoTw37Ysr- |
GitHub Actions
Inactive
|
iOS Build Available Rocket.Chat Experimental 4.64.0.98157 |
GitHub Actions
Error
GitHub Actions
Error
GitHub Actions
Error
GitHub Actions
Error
GitHub Actions
Error
GitHub Actions
Error
GitHub Actions
Error
GitHub Actions
Error
GitHub Actions
Error
GitHub Actions
Error
GitHub Actions
Error
GitHub Actions
Error
GitHub Actions
Inactive
GitHub Actions
Error
GitHub Actions
Error
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
app/containers/ActionSheet/Item.tsx (1)
63-63: Simplify conditional rendering of right contentMinor cleanup to avoid duplicated checks.
- {item.right ? <View style={styles.rightContainer}>{item.right ? item.right() : null}</View> : null} + {item.right && <View style={styles.rightContainer}>{item.right()}</View>}app/i18n/locales/ru.json (1)
7-11: RU phrasing and consistency — suggest improved translationsMore natural Russian and consistent punctuation. “Отклонение” ≠ “dismiss”; prefer “закрытие”. “Тосты” can be confusing; “Всплывающие уведомления” is clearer.
- "A11y_appearance_dialog_require_manual_dismissal": "Требуется ручное отклонение.", - "A11y_appearance_dialogs": "Диалоги", - "A11y_appearance_show_alerts_as": "Показывать уведомления как", - "A11y_appearance_toast_dismissed_automatically": "Отклонено автоматически", - "A11y_appearance_toasts": "Тосты", + "A11y_appearance_dialog_require_manual_dismissal": "Требуется закрыть вручную", + "A11y_appearance_dialogs": "Диалоговые окна", + "A11y_appearance_show_alerts_as": "Показывать уведомления в виде", + "A11y_appearance_toast_dismissed_automatically": "Закрывается автоматически", + "A11y_appearance_toasts": "Всплывающие уведомления",app/views/AccessibilityAndAppearanceView/components/ListPicker.tsx (1)
88-93: Enhance accessibility label with selection detailInclude the option’s description so SR users hear auto/manual dismissal behavior on the settings row too.
- accessibilityLabel={`${title}. ${option?.label}`} + accessibilityLabel={`${title}. ${option?.label}${option?.description ? `. ${option.description}` : ''}`}Also applies to: 96-100
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (33)
.maestro/tests/accessibilityAndAppearance/ToastsAndDialogs.yml(1 hunks)app/containers/ActionSheet/Item.tsx(1 hunks)app/containers/ActionSheet/Provider.tsx(1 hunks)app/containers/ActionSheet/styles.ts(1 hunks)app/containers/Toast.tsx(2 hunks)app/i18n/locales/ar.json(1 hunks)app/i18n/locales/bn-IN.json(1 hunks)app/i18n/locales/cs.json(1 hunks)app/i18n/locales/de.json(1 hunks)app/i18n/locales/en.json(1 hunks)app/i18n/locales/es.json(1 hunks)app/i18n/locales/fi.json(1 hunks)app/i18n/locales/fr.json(1 hunks)app/i18n/locales/hi-IN.json(1 hunks)app/i18n/locales/hu.json(1 hunks)app/i18n/locales/it.json(1 hunks)app/i18n/locales/ja.json(1 hunks)app/i18n/locales/nl.json(1 hunks)app/i18n/locales/nn.json(1 hunks)app/i18n/locales/no.json(1 hunks)app/i18n/locales/pt-BR.json(1 hunks)app/i18n/locales/pt-PT.json(1 hunks)app/i18n/locales/ru.json(1 hunks)app/i18n/locales/sl-SI.json(1 hunks)app/i18n/locales/sv.json(1 hunks)app/i18n/locales/ta-IN.json(1 hunks)app/i18n/locales/te-IN.json(1 hunks)app/i18n/locales/tr.json(1 hunks)app/i18n/locales/zh-CN.json(1 hunks)app/i18n/locales/zh-TW.json(1 hunks)app/lib/constants/keys.ts(1 hunks)app/views/AccessibilityAndAppearanceView/components/ListPicker.tsx(1 hunks)app/views/AccessibilityAndAppearanceView/index.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (30)
- app/containers/Toast.tsx
- app/i18n/locales/en.json
- app/i18n/locales/sl-SI.json
- app/i18n/locales/nl.json
- app/containers/ActionSheet/styles.ts
- app/i18n/locales/de.json
- app/views/AccessibilityAndAppearanceView/index.tsx
- app/i18n/locales/zh-CN.json
- app/i18n/locales/nn.json
- app/i18n/locales/es.json
- app/i18n/locales/pt-BR.json
- app/containers/ActionSheet/Provider.tsx
- app/i18n/locales/sv.json
- app/i18n/locales/hu.json
- app/i18n/locales/pt-PT.json
- app/i18n/locales/it.json
- app/i18n/locales/hi-IN.json
- .maestro/tests/accessibilityAndAppearance/ToastsAndDialogs.yml
- app/i18n/locales/fr.json
- app/i18n/locales/ar.json
- app/i18n/locales/te-IN.json
- app/i18n/locales/ja.json
- app/i18n/locales/bn-IN.json
- app/i18n/locales/zh-TW.json
- app/lib/constants/keys.ts
- app/i18n/locales/no.json
- app/i18n/locales/cs.json
- app/i18n/locales/fi.json
- app/i18n/locales/ta-IN.json
- app/i18n/locales/tr.json
🧰 Additional context used
🧬 Code graph analysis (1)
app/views/AccessibilityAndAppearanceView/components/ListPicker.tsx (3)
app/views/AccessibilityAndAppearanceView/index.tsx (1)
TAlertDisplayType(21-21)app/containers/ActionSheet/Provider.tsx (2)
useActionSheet(41-41)TActionSheetOptionsItem(7-17)app/theme.tsx (1)
useTheme(29-29)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ESLint and Test / run-eslint-and-test
🔇 Additional comments (6)
app/containers/ActionSheet/Item.tsx (4)
39-39: Compose accessibilityLabel safely; avoid trailing punctuation and||pitfallsUse nullish coalescing and join only defined parts to prevent treating an empty custom label as falsy and to avoid trailing punctuation/spaces.
- const accessibilityLabel = item?.accessibilityLabel || `${item.title}. ${item?.subtitle || ''}`; + const accessibilityLabel = item?.accessibilityLabel ?? [item.title, item?.subtitle].filter(Boolean).join(', ');
43-49: Expose disabled state to screen readersAnnounce disabled state; keep press handling as-is so you can show the permission message.
<Touch accessible accessibilityLabel={accessibilityLabel} accessibilityRole='button' + accessibilityState={{ disabled: !enabled }} onPress={onPress} - style={[styles.item, { backgroundColor: colors.surfaceLight, height }]} + style={[styles.item, { backgroundColor: colors.surfaceLight, height }]} testID={item.testID}>Optionally, if you prefer non-interactive behavior: add
disabled={!enabled}too.
38-48: Use minHeight instead of a fixed height for better Dynamic Type supportAvoid shrinking below the base size on small font scales and allow content to grow.
- const height = 48 * fontScale; + const minHeight = Math.max(48, 48 * fontScale); ... - style={[styles.item, { backgroundColor: colors.surfaceLight, height }]} + style={[styles.item, { backgroundColor: colors.surfaceLight, minHeight }]}
55-61: Subtitle rendering looks goodGood use of theme color and spacing; single-line constraint matches title behavior.
app/views/AccessibilityAndAppearanceView/components/ListPicker.tsx (2)
53-56: Type safety for value — good fixProp value now uses TAlertDisplayType. Matches onChangeValue and prevents invalid states.
88-103: Incorrect — the prop is defined & used asadditionalAcessibilityLabelin this repoThe prop is declared and consumed as
additionalAcessibilityLabelin app/containers/List/ListItem.tsx and appears in many call sites; changing toadditionalAccessibilityLabelwould require a central API rename (or an alias) and updates across files.Likely an incorrect or invalid review comment.
|
Android Build Available Rocket.Chat Experimental 4.65.0.107393 Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNQC-YMv_o1HP-NlAmn_ZgRlsQecpsQ953eNo46rxMcNogQsVLDHDEzRMDoZYYAy9GmB1zXdWC5_i-FRTGJf |
|
iOS Build Available Rocket.Chat Experimental 4.65.0.107395 |
There was a problem hiding this comment.
Actionable comments posted: 11
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (16)
app/i18n/locales/ar.json(1 hunks)app/i18n/locales/cs.json(1 hunks)app/i18n/locales/de.json(1 hunks)app/i18n/locales/hu.json(1 hunks)app/i18n/locales/it.json(1 hunks)app/i18n/locales/nn.json(1 hunks)app/i18n/locales/pt-BR.json(1 hunks)app/i18n/locales/pt-PT.json(1 hunks)app/i18n/locales/sl-SI.json(1 hunks)app/i18n/locales/sv.json(1 hunks)app/i18n/locales/ta-IN.json(1 hunks)app/i18n/locales/te-IN.json(1 hunks)app/i18n/locales/tr.json(1 hunks)app/i18n/locales/zh-CN.json(1 hunks)app/i18n/locales/zh-TW.json(1 hunks)app/views/AccessibilityAndAppearanceView/components/ListPicker.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- app/i18n/locales/zh-CN.json
- app/i18n/locales/nn.json
- app/views/AccessibilityAndAppearanceView/components/ListPicker.tsx
- app/i18n/locales/pt-PT.json
🔇 Additional comments (1)
app/i18n/locales/ta-IN.json (1)
7-11: Refine the Tamil terminology for dialogs and toastsThese strings still use direct transliterations (“டையலாக்கள்”, “டோஸ்ட்ஸ்”) and “கைமுறையாக நிராகரிக்க வேண்டும்”, which read unnatural in Tamil UI copy. Please adopt the idiomatic phrasing suggested earlier.
- "A11y_appearance_dialog_require_manual_dismissal": "கைமுறையாக நிராகரிக்க வேண்டும்", - "A11y_appearance_dialogs": "டையலாக்கள்", - "A11y_appearance_show_alerts_as": "அறிவிப்புகளை இவ்வாறு காட்டு:", - "A11y_appearance_toast_dismissed_automatically": "தானாகவே தள்ளுபடி செய்யப்பட்டது", - "A11y_appearance_toasts": "டோஸ்ட்ஸ்", + "A11y_appearance_dialog_require_manual_dismissal": "கைமுறையாக மூட வேண்டும்", + "A11y_appearance_dialogs": "உரையாடல் பெட்டிகள்", + "A11y_appearance_show_alerts_as": "எச்சரிக்கைகளை இவ்வாறு காட்டு:", + "A11y_appearance_toast_dismissed_automatically": "தானாக மூடப்படும்", + "A11y_appearance_toasts": "டோஸ்ட் அறிவிப்புகள்",
|
iOS Build Available Rocket.Chat Experimental 4.65.0.107424 |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/i18n/locales/cs.json (1)
7-11: Polish the Czech wording.The sentences currently mix styles (“Vyžaduje ….” with trailing period, “Toast upozornění”). Please align with common UI phrasing:
- "A11y_appearance_dialog_require_manual_dismissal": "Vyžaduje ruční zavření.", + "A11y_appearance_dialog_require_manual_dismissal": "Vyžaduje ruční zavření", ... - "A11y_appearance_toasts": "Toast upozornění", + "A11y_appearance_toasts": "Toastová upozornění",That keeps the strings stylistically consistent with surrounding keys and sounds natural.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
app/i18n/locales/ar.json(1 hunks)app/i18n/locales/cs.json(1 hunks)app/i18n/locales/de.json(1 hunks)app/i18n/locales/hu.json(1 hunks)app/i18n/locales/sl-SI.json(1 hunks)app/i18n/locales/sv.json(1 hunks)app/i18n/locales/te-IN.json(1 hunks)app/i18n/locales/tr.json(1 hunks)app/i18n/locales/zh-TW.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- app/i18n/locales/sv.json
- app/i18n/locales/ar.json
- app/i18n/locales/zh-TW.json
- app/i18n/locales/hu.json
🔇 Additional comments (2)
app/i18n/locales/te-IN.json (1)
7-11: Nice cleanup on the Telugu strings.The new labels are now fully in Telugu, and “టోస్ట్ నోటిఫికేషన్లు” reads clearly in the UI.
app/i18n/locales/tr.json (1)
3-7: Turkish phrasing looks good.“Toast bildirimleri” and the rest of the copy now read naturally for native speakers.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
app/views/AccessibilityAndAppearanceView/components/ListPicker.tsx (3)
71-82: Tighten a11y label construction and remove unnecessary optional chainingAvoid producing a stray period when description is empty and drop optional chaining on non-null variables.
-const getOptions = (): TActionSheetOptionsItem[] => - OPTIONS.map(i => ({ +const getOptions = (): TActionSheetOptionsItem[] => + OPTIONS.map(i => ({ title: i.label, - subtitle: i?.description || undefined, - accessibilityLabel: `${i.label}. ${i?.description || ''}. ${ - option?.value === i.value ? I18n.t('Checked') : I18n.t('Unchecked') - }`, + subtitle: i.description || undefined, + accessibilityLabel: `${i.label}${i.description ? `. ${i.description}` : ''}. ${option.value === i.value ? I18n.t('Checked') : I18n.t('Unchecked')}`, onPress: () => { hideActionSheet(); onChangeValue(i.value); }, - right: option?.value === i.value ? () => <CustomIcon name={'check'} size={20} color={colors.strokeHighlight} /> : undefined + right: option.value === i.value ? () => <CustomIcon name="check" size={20} color={colors.strokeHighlight} /> : undefined }));
71-82: Add testIDs to ActionSheet items for E2E reliability (Maestro)Helps target options deterministically in tests.
OPTIONS.map(i => ({ title: i.label, + testID: `listpicker-option-${i.value.toLowerCase()}`,
58-65: Drop unnecessary type assertionsString literals already satisfy the union; the casts add noise without benefit.
- value: 'TOAST' as TAlertDisplayType, + value: 'TOAST', ... - value: 'DIALOG' as TAlertDisplayType, + value: 'DIALOG',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
app/views/AccessibilityAndAppearanceView/components/ListPicker.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/views/AccessibilityAndAppearanceView/components/ListPicker.tsx (3)
app/views/AccessibilityAndAppearanceView/index.tsx (1)
TAlertDisplayType(21-21)app/containers/ActionSheet/Provider.tsx (2)
useActionSheet(41-41)TActionSheetOptionsItem(7-17)app/theme.tsx (1)
useTheme(29-29)
🔇 Additional comments (2)
app/views/AccessibilityAndAppearanceView/components/ListPicker.tsx (2)
55-66: Ensure i18n-driven text updates when locale changesIf locale changes outside React’s lifecycle, these labels may not re-render (previously raised in review). Verify this component re-renders on locale change; if not, subscribe to i18n change events or use a translation hook/context that triggers updates.
If needed, I can propose a small hook to subscribe to your i18n’s “locale changed” event and force a re-render.
Also applies to: 68-68, 90-101
103-104: Ignore incorrect prop rename suggestion
The propadditionalAcessibilityLabel(single-c) is defined and consumed in app/containers/List/ListItem.tsx, so renaming it here would break its implementation. Leave it as is.Likely an incorrect or invalid review comment.
2 participants
Proposed changes
Introduces a picker to select how alerts are displayed (Toasts or Native dialogs).
Issue(s)
https://rocketchat.atlassian.net/browse/MA-105
How to test or reproduce
Screenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit
New Features
Accessibility
Localization
Tests